From 8bb643976fe20d1ec684291aa7bf5337e474bec4 Mon Sep 17 00:00:00 2001
From: Oliver Woodman <olly@google.com>
Date: Fri, 23 Jun 2017 17:20:51 +0100
Subject: [PATCH] Fix ContentDataSource and enhance tests to validate read data

---
 .../core/src/androidTest/AndroidManifest.xml  |   4 +-
 .../assets/binary/1024_incrementing_bytes.mp3 | Bin 0 -> 1024 bytes
 .../upstream/AndroidDataSourceConstants.java  |  16 --
 .../upstream/AssetDataSourceTest.java         |  59 ++++++--
 .../upstream/ContentDataSourceTest.java       | 139 ++++++++++++++----
 .../exoplayer2/upstream/TestDataProvider.java |  68 ---------
 .../upstream/ContentDataSource.java           |   5 +-
 .../android/exoplayer2/testutil/TestUtil.java |  17 +++
 8 files changed, 185 insertions(+), 123 deletions(-)
 create mode 100644 library/core/src/androidTest/assets/binary/1024_incrementing_bytes.mp3
 delete mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java
 delete mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java

diff --git a/library/core/src/androidTest/AndroidManifest.xml b/library/core/src/androidTest/AndroidManifest.xml
index 0a90b071d2..aeddc611cf 100644
--- a/library/core/src/androidTest/AndroidManifest.xml
+++ b/library/core/src/androidTest/AndroidManifest.xml
@@ -25,8 +25,8 @@
       tools:ignore="MissingApplicationIcon,HardcodedDebugMode">
     <uses-library android:name="android.test.runner"/>
     <provider
-      android:authorities="exoplayer"
-      android:name="com.google.android.exoplayer2.upstream.TestDataProvider"/>
+      android:authorities="com.google.android.exoplayer2.core.test"
+      android:name="com.google.android.exoplayer2.upstream.ContentDataSourceTest$TestContentProvider"/>
   </application>
 
   <instrumentation
diff --git a/library/core/src/androidTest/assets/binary/1024_incrementing_bytes.mp3 b/library/core/src/androidTest/assets/binary/1024_incrementing_bytes.mp3
new file mode 100644
index 0000000000000000000000000000000000000000..c8b49c8cd518e58491924bfc364ff26e01a85009
GIT binary patch
literal 1024
zcmZQzWMXDvWn<^y<l^Sx<>MC+6cQE@6%&_`l#-T_m6KOcR8m$^Ra4i{)Y8_`)zddH
zG%_|ZH8Z!cw6eCbwX=6{baHlab#wRd^z!!c_45x13<?ej4GWKmjEatljf+o6OiE5k
zO-s+n%*xKm&C4$+EGjN3Ei136tg5c5t*dWnY-(<4ZENr7?CS36?dzW~anj@|Q>RUz
zF>}`JIdkXDU$Ah|;w4L$Enl&6)#^2C*R9{Mant54TeofBv2)k%J$v`<KXCBS;Uh<n
z9Y1mM)af&4&z-+;@zUihSFc^aar4&gJ9qEhfAH|p<0ns_J%91?)$2EJ-@X6v@zduo
aU%!3-@$=X3KY#!IXBhSWX#9`h^bY{!$^H-k

literal 0
HcmV?d00001

diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java
deleted file mode 100644
index d11202ccf2..0000000000
--- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java
+++ /dev/null
@@ -1,16 +0,0 @@
-package com.google.android.exoplayer2.upstream;
-
-import android.content.ContentResolver;
-import android.net.Uri;
-
-final class AndroidDataSourceConstants {
-  static final long SAMPLE_MP4_BYTES = 101597;
-  static final String SAMPLE_MP4_PATH = "/mp4/sample.mp4";
-  static final String TEST_DATA_PROVIDER_AUTHORITY = "exoplayer";
-  static final Uri NULL_DESCRIPTOR_URI = new Uri.Builder()
-          .scheme(ContentResolver.SCHEME_CONTENT)
-          .authority(TEST_DATA_PROVIDER_AUTHORITY)
-          .build();
-
-  private AndroidDataSourceConstants() {}
-}
diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java
index 178842bd4d..d8e61eb94c 100644
--- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java
+++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java
@@ -1,21 +1,62 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package com.google.android.exoplayer2.upstream;
 
 import android.content.Context;
 import android.net.Uri;
 import android.test.InstrumentationTestCase;
+import android.test.MoreAsserts;
+import com.google.android.exoplayer2.testutil.TestUtil;
 
-import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_BYTES;
-import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_PATH;
+/**
+ * Unit tests for {@link AssetDataSource}.
+ */
+public final class AssetDataSourceTest extends InstrumentationTestCase {
 
-public class AssetDataSourceTest extends InstrumentationTestCase {
+  private static final String DATA_PATH = "binary/1024_incrementing_bytes.mp3";
+  private static final long DATA_LENGTH = 1024;
 
-  public void testAssetDataSource() throws Exception {
-    final Context context = getInstrumentation().getContext();
+  public void testReadFileUri() throws Exception {
+    Context context = getInstrumentation().getContext();
     AssetDataSource dataSource = new AssetDataSource(context);
-    Uri assetUri = Uri.parse("file:///android_asset" + SAMPLE_MP4_PATH);
+    Uri assetUri = Uri.parse("file:///android_asset/" + DATA_PATH);
     DataSpec dataSpec = new DataSpec(assetUri);
-    long sourceLengthBytes = dataSource.open(dataSpec);
-
-    assertEquals(SAMPLE_MP4_BYTES, sourceLengthBytes);
+    try {
+      long length = dataSource.open(dataSpec);
+      assertEquals(DATA_LENGTH, length);
+      byte[] readData = TestUtil.readToEnd(dataSource);
+      MoreAsserts.assertEquals(TestUtil.getByteArray(getInstrumentation(), DATA_PATH), readData);
+    } finally {
+      dataSource.close();
+    }
   }
+
+  public void testReadAssetUri() throws Exception {
+    Context context = getInstrumentation().getContext();
+    AssetDataSource dataSource = new AssetDataSource(context);
+    Uri assetUri = Uri.parse("asset:///" + DATA_PATH);
+    DataSpec dataSpec = new DataSpec(assetUri);
+    try {
+      long length = dataSource.open(dataSpec);
+      assertEquals(DATA_LENGTH, length);
+      byte[] readData = TestUtil.readToEnd(dataSource);
+      MoreAsserts.assertEquals(TestUtil.getByteArray(getInstrumentation(), DATA_PATH), readData);
+    } finally {
+      dataSource.close();
+    }
+  }
+
 }
diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java
index 1cd14b45e1..10a408c578 100644
--- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java
+++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java
@@ -1,40 +1,127 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package com.google.android.exoplayer2.upstream;
 
+import android.content.ContentProvider;
 import android.content.ContentResolver;
-import android.content.Context;
+import android.content.ContentValues;
+import android.content.res.AssetFileDescriptor;
+import android.database.Cursor;
 import android.net.Uri;
+import android.support.annotation.NonNull;
 import android.test.InstrumentationTestCase;
+import android.test.MoreAsserts;
+import com.google.android.exoplayer2.testutil.TestUtil;
+import java.io.FileNotFoundException;
+import java.io.IOException;
 
-import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.NULL_DESCRIPTOR_URI;
-import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_BYTES;
-import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_PATH;
-import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.TEST_DATA_PROVIDER_AUTHORITY;
+/**
+ * Unit tests for {@link ContentDataSource}.
+ */
+public final class ContentDataSourceTest extends InstrumentationTestCase {
 
-public class ContentDataSourceTest extends InstrumentationTestCase {
+  private static final String DATA_PATH = "binary/1024_incrementing_bytes.mp3";
+  private static final long DATA_LENGTH = 1024;
 
-  public void testValidContentDataSource() throws Exception {
-    Context context = getInstrumentation().getContext();
-    ContentDataSource dataSource = new ContentDataSource(context);
+  public void testReadValidUri() throws Exception {
+    ContentDataSource dataSource = new ContentDataSource(getInstrumentation().getContext());
     Uri contentUri = new Uri.Builder()
-            .scheme(ContentResolver.SCHEME_CONTENT)
-            .authority(TEST_DATA_PROVIDER_AUTHORITY)
-            .path(SAMPLE_MP4_PATH).build();
+        .scheme(ContentResolver.SCHEME_CONTENT)
+        .authority(TestContentProvider.AUTHORITY)
+        .path(DATA_PATH).build();
     DataSpec dataSpec = new DataSpec(contentUri);
-    long sourceLengthBytes = dataSource.open(dataSpec);
-
-    assertEquals(SAMPLE_MP4_BYTES, sourceLengthBytes);
-  }
-
-  public void testNullContentDataSource() throws Exception {
-    Context context = getInstrumentation().getContext();
-    ContentDataSource dataSource = new ContentDataSource(context);
-    DataSpec dataSpec = new DataSpec(NULL_DESCRIPTOR_URI);
-
     try {
-      dataSource.open(dataSpec);
-      fail("Expected exception not thrown.");
-    } catch (ContentDataSource.ContentDataSourceException e) {
-      // Expected.
+      long length = dataSource.open(dataSpec);
+      assertEquals(DATA_LENGTH, length);
+      byte[] readData = TestUtil.readToEnd(dataSource);
+      MoreAsserts.assertEquals(TestUtil.getByteArray(getInstrumentation(), DATA_PATH), readData);
+    } finally {
+      dataSource.close();
     }
   }
+
+  public void testReadInvalidUri() throws Exception {
+    ContentDataSource dataSource = new ContentDataSource(getInstrumentation().getContext());
+    Uri contentUri = new Uri.Builder()
+        .scheme(ContentResolver.SCHEME_CONTENT)
+        .authority(TestContentProvider.AUTHORITY)
+        .build();
+    DataSpec dataSpec = new DataSpec(contentUri);
+    try {
+      dataSource.open(dataSpec);
+      fail();
+    } catch (ContentDataSource.ContentDataSourceException e) {
+      // Expected.
+    } finally {
+      dataSource.close();
+    }
+  }
+
+  public static final class TestContentProvider extends ContentProvider {
+
+    private static final String AUTHORITY = "com.google.android.exoplayer2.core.test";
+
+    @Override
+    public boolean onCreate() {
+      return true;
+    }
+
+    @Override
+    public Cursor query(@NonNull Uri uri, String[] projection, String selection,
+        String[] selectionArgs, String sortOrder) {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public AssetFileDescriptor openAssetFile(@NonNull Uri uri, @NonNull String mode)
+        throws FileNotFoundException {
+      if (uri.getPath() == null) {
+        return null;
+      }
+      try {
+        return getContext().getAssets().openFd(uri.getPath().replaceFirst("/", ""));
+      } catch (IOException e) {
+        FileNotFoundException exception = new FileNotFoundException(e.getMessage());
+        exception.initCause(e);
+        throw exception;
+      }
+    }
+
+    @Override
+    public String getType(@NonNull Uri uri) {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Uri insert(@NonNull Uri uri, ContentValues values) {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public int delete(@NonNull Uri uri, String selection,
+        String[] selectionArgs) {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public int update(@NonNull Uri uri, ContentValues values,
+        String selection, String[] selectionArgs) {
+      throw new UnsupportedOperationException();
+    }
+
+  }
+
 }
diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java
deleted file mode 100644
index f6e09a7067..0000000000
--- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java
+++ /dev/null
@@ -1,68 +0,0 @@
-package com.google.android.exoplayer2.upstream;
-
-import android.content.ContentProvider;
-import android.content.ContentValues;
-import android.content.Context;
-import android.content.res.AssetFileDescriptor;
-import android.database.Cursor;
-import android.net.Uri;
-import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
-
-import java.io.FileNotFoundException;
-import java.io.IOException;
-
-import static junit.framework.Assert.assertNotNull;
-
-public class TestDataProvider extends ContentProvider {
-  @Override
-  public boolean onCreate() {
-    return true;
-  }
-
-  @Nullable
-  @Override
-  public Cursor query(@NonNull final Uri uri, @Nullable final String[] projection, @Nullable final String selection, @Nullable final String[] selectionArgs, @Nullable final String sortOrder) {
-    throw new UnsupportedOperationException("Not implemented");
-  }
-
-  @Nullable
-  @Override
-  public AssetFileDescriptor openAssetFile(@NonNull final Uri uri, @NonNull final String mode) throws FileNotFoundException {
-    if (uri.equals(AndroidDataSourceConstants.NULL_DESCRIPTOR_URI)) {
-      return null;
-    }
-
-    try {
-      Context context = getContext();
-      assertNotNull(context);
-      return context.getAssets().openFd(uri.getPath().replaceFirst("/", ""));
-    } catch (IOException e) {
-      FileNotFoundException exception = new FileNotFoundException(e.getMessage());
-      exception.initCause(e);
-      throw exception;
-    }
-  }
-
-  @Nullable
-  @Override
-  public String getType(@NonNull final Uri uri) {
-    throw new UnsupportedOperationException("Not implemented");
-  }
-
-  @Nullable
-  @Override
-  public Uri insert(@NonNull final Uri uri, @Nullable final ContentValues values) {
-    throw new UnsupportedOperationException("Not implemented");
-  }
-
-  @Override
-  public int delete(@NonNull final Uri uri, @Nullable final String selection, @Nullable final String[] selectionArgs) {
-    throw new UnsupportedOperationException("Not implemented");
-  }
-
-  @Override
-  public int update(@NonNull final Uri uri, @Nullable final ContentValues values, @Nullable final String selection, @Nullable final String[] selectionArgs) {
-    throw new UnsupportedOperationException("Not implemented");
-  }
-}
diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java
index 3a9be552d2..507162519a 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java
@@ -76,8 +76,9 @@ public final class ContentDataSource implements DataSource {
         throw new FileNotFoundException("Could not open file descriptor for: " + uri);
       }
       inputStream = new FileInputStream(assetFileDescriptor.getFileDescriptor());
-      long skipped = inputStream.skip(dataSpec.position);
-      if (skipped < dataSpec.position) {
+      long assertStartOffset = assetFileDescriptor.getStartOffset();
+      long skipped = inputStream.skip(assertStartOffset + dataSpec.position) - assertStartOffset;
+      if (skipped != dataSpec.position) {
         // We expect the skip to be satisfied in full. If it isn't then we're probably trying to
         // skip beyond the end of the data.
         throw new EOFException();
diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java
index 75a4a01923..f75239318a 100644
--- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java
+++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java
@@ -22,6 +22,7 @@ import com.google.android.exoplayer2.extractor.Extractor;
 import com.google.android.exoplayer2.extractor.PositionHolder;
 import com.google.android.exoplayer2.extractor.SeekMap;
 import com.google.android.exoplayer2.testutil.FakeExtractorInput.SimulatedIOException;
+import com.google.android.exoplayer2.upstream.DataSource;
 import com.google.android.exoplayer2.util.Assertions;
 import com.google.android.exoplayer2.util.Util;
 import java.io.IOException;
@@ -64,6 +65,22 @@ public class TestUtil {
     }
   }
 
+  public static byte[] readToEnd(DataSource dataSource) throws IOException {
+    byte[] data = new byte[1024];
+    int position = 0;
+    int bytesRead = 0;
+    while (bytesRead != C.RESULT_END_OF_INPUT) {
+      if (position == data.length) {
+        data = Arrays.copyOf(data, data.length * 2);
+      }
+      bytesRead = dataSource.read(data, position, data.length - position);
+      if (bytesRead != C.RESULT_END_OF_INPUT) {
+        position += bytesRead;
+      }
+    }
+    return Arrays.copyOf(data, position);
+  }
+
   public static FakeExtractorOutput consumeTestData(Extractor extractor, FakeExtractorInput input,
       long timeUs) throws IOException, InterruptedException {
     return consumeTestData(extractor, input, timeUs, false);