Use DownloadAction.id properly

- Fix ID being dropped when DownloadAction is serialized and
  de-serialized as it's passed to DownloadService
- Properly set DownloadAction.id when building an action from
  a DownloadState
- Make ID a required constructor argument. Else it's too easy
  to not propagate it by accident.

PiperOrigin-RevId: 242457831
This commit is contained in:
olly 2019-04-08 15:15:11 +01:00 committed by Oliver Woodman
parent 401e20d9c8
commit 47601980d9
13 changed files with 54 additions and 38 deletions

View file

@ -45,7 +45,7 @@ public final class DownloadAction {
/** Type for SmoothStreaming downloads. */
public static final String TYPE_SS = "ss";
private static final int VERSION = 2;
private static final int VERSION = 3;
/**
* Deserializes an action from the {@code data}.
@ -78,25 +78,6 @@ public final class DownloadAction {
return readFromStream(new DataInputStream(input));
}
/**
* Creates a DASH download action.
*
* @param type The type of the action.
* @param uri The URI of the media to be downloaded.
* @param keys Keys of streams to be downloaded. If empty, all streams will be downloaded.
* @param customCacheKey A custom key for cache indexing, or null.
* @param data Optional custom data for this action. If {@code null} an empty array will be used.
*/
public static DownloadAction createDownloadAction(
String type,
Uri uri,
List<StreamKey> keys,
@Nullable String customCacheKey,
@Nullable byte[] data) {
return createDownloadAction(
generateId(uri, customCacheKey), type, uri, keys, customCacheKey, data);
}
/**
* Creates a DASH download action.
*
@ -224,6 +205,7 @@ public final class DownloadAction {
if (customCacheKey != null) {
dataOutputStream.writeUTF(customCacheKey);
}
dataOutputStream.writeUTF(id);
dataOutputStream.flush();
}
@ -261,15 +243,17 @@ public final class DownloadAction {
customCacheKey = input.readBoolean() ? input.readUTF() : null;
}
// Serialized version 0, 1 and 2 did not contain an id.
String id = version < 3 ? generateId(uri, customCacheKey) : input.readUTF();
if (isRemoveAction) {
// Remove actions are not supported anymore.
throw new UnsupportedActionException();
}
return new DownloadAction(
generateId(uri, customCacheKey), type, uri, keys, customCacheKey, data);
return new DownloadAction(id, type, uri, keys, customCacheKey, data);
}
private static String generateId(Uri uri, @Nullable String customCacheKey) {
/* package */ static String generateId(Uri uri, @Nullable String customCacheKey) {
return customCacheKey != null ? customCacheKey : uri.toString();
}

View file

@ -584,9 +584,10 @@ public final class DownloadHelper {
* @return The built {@link DownloadAction}.
*/
public DownloadAction getDownloadAction(@Nullable byte[] data) {
String downloadId = uri.toString();
if (mediaSource == null) {
return DownloadAction.createDownloadAction(
downloadType, uri, /* keys= */ Collections.emptyList(), cacheKey, data);
downloadId, downloadType, uri, /* keys= */ Collections.emptyList(), cacheKey, data);
}
assertPreparedWithMedia();
List<StreamKey> streamKeys = new ArrayList<>();
@ -600,7 +601,8 @@ public final class DownloadHelper {
}
streamKeys.addAll(mediaPreparer.mediaPeriods[periodIndex].getStreamKeys(allSelections));
}
return DownloadAction.createDownloadAction(downloadType, uri, streamKeys, cacheKey, data);
return DownloadAction.createDownloadAction(
downloadId, downloadType, uri, streamKeys, cacheKey, data);
}
// Initialization of array of Lists.

View file

@ -886,6 +886,7 @@ public final class DownloadManager {
public DownloadAction getAction() {
return DownloadAction.createDownloadAction(
downloadState.id,
downloadState.type,
downloadState.uri,
Arrays.asList(downloadState.streamKeys),

View file

@ -171,6 +171,6 @@ public class ActionFileTest {
private static DownloadAction buildAction(String type, Uri uri, byte[] data) {
return DownloadAction.createDownloadAction(
type, uri, /* keys= */ Collections.emptyList(), /* customCacheKey= */ null, data);
"id", type, uri, /* keys= */ Collections.emptyList(), /* customCacheKey= */ null, data);
}
}

View file

@ -39,6 +39,7 @@ public final class DefaultDownloaderFactoryTest {
Downloader downloader =
factory.createDownloader(
DownloadAction.createDownloadAction(
"id",
DownloadAction.TYPE_PROGRESSIVE,
Uri.parse("https://www.test.com/download"),
/* keys= */ Collections.emptyList(),

View file

@ -84,13 +84,6 @@ public class DownloadActionTest {
assertThat(action1.id.equals(action2.id)).isTrue();
}
@Test
public void testDifferentCacheKeyDifferentUri_hasDifferentId() {
DownloadAction action1 = createAction(uri1, "key123");
DownloadAction action2 = createAction(uri2, "key456");
assertThat(action1.id.equals(action2.id)).isFalse();
}
@SuppressWarnings("EqualsWithItself")
@Test
public void testEquals() {
@ -130,6 +123,7 @@ public class DownloadActionTest {
public void testSerializerWriteRead() throws Exception {
assertStreamSerializationRoundTrip(
DownloadAction.createDownloadAction(
"id",
TYPE_DASH,
uri1,
toList(new StreamKey(0, 1, 2), new StreamKey(3, 4, 5)),
@ -142,6 +136,7 @@ public class DownloadActionTest {
public void testArraySerialization() throws Exception {
assertArraySerializationRoundTrip(
DownloadAction.createDownloadAction(
"id",
TYPE_DASH,
uri1,
toList(new StreamKey(0, 1, 2), new StreamKey(3, 4, 5)),
@ -152,10 +147,12 @@ public class DownloadActionTest {
@Test
public void testSerializerProgressiveVersion0() throws Exception {
String customCacheKey = "key123";
String expectedId = DownloadAction.generateId(uri1, customCacheKey);
assertDeserialization(
"progressive-download-v0",
DownloadAction.createDownloadAction(
TYPE_PROGRESSIVE, uri1, Collections.emptyList(), "key123", data));
expectedId, TYPE_PROGRESSIVE, uri1, Collections.emptyList(), customCacheKey, data));
assertUnsupportedAction("progressive-remove-v0");
}
@ -164,6 +161,7 @@ public class DownloadActionTest {
assertDeserialization(
"dash-download-v0",
DownloadAction.createDownloadAction(
uri1.toString(),
TYPE_DASH,
uri1,
toList(new StreamKey(0, 1, 2), new StreamKey(3, 4, 5)),
@ -177,6 +175,7 @@ public class DownloadActionTest {
assertDeserialization(
"hls-download-v0",
DownloadAction.createDownloadAction(
uri1.toString(),
TYPE_HLS,
uri1,
toList(new StreamKey(0, 1), new StreamKey(2, 3)),
@ -190,6 +189,7 @@ public class DownloadActionTest {
assertDeserialization(
"hls-download-v1",
DownloadAction.createDownloadAction(
uri1.toString(),
TYPE_HLS,
uri1,
toList(new StreamKey(0, 1, 2), new StreamKey(3, 4, 5)),
@ -203,6 +203,7 @@ public class DownloadActionTest {
assertDeserialization(
"ss-download-v0",
DownloadAction.createDownloadAction(
uri1.toString(),
TYPE_SS,
uri1,
toList(new StreamKey(0, 1), new StreamKey(2, 3)),
@ -216,6 +217,7 @@ public class DownloadActionTest {
assertDeserialization(
"ss-download-v1",
DownloadAction.createDownloadAction(
uri1.toString(),
TYPE_SS,
uri1,
toList(new StreamKey(0, 1, 2), new StreamKey(3, 4, 5)),
@ -226,12 +228,17 @@ public class DownloadActionTest {
private DownloadAction createAction(Uri uri, StreamKey... keys) {
return DownloadAction.createDownloadAction(
TYPE_DASH, uri, toList(keys), /* customCacheKey= */ null, data);
uri.toString(), TYPE_DASH, uri, toList(keys), /* customCacheKey= */ null, data);
}
private DownloadAction createAction(Uri uri, @Nullable String customCacheKey) {
return DownloadAction.createDownloadAction(
DownloadAction.TYPE_DASH, uri, Collections.emptyList(), customCacheKey, /* data= */ null);
"id",
DownloadAction.TYPE_DASH,
uri,
Collections.emptyList(),
customCacheKey,
/* data= */ null);
}
private static void assertNotEqual(DownloadAction action1, DownloadAction action2) {

View file

@ -58,6 +58,7 @@ public class DownloadIndexUtilTest {
byte[] data = new byte[] {1, 2, 3, 4};
DownloadAction action =
DownloadAction.createDownloadAction(
"id",
TYPE_DASH,
Uri.parse("https://www.test.com/download"),
asList(
@ -79,6 +80,7 @@ public class DownloadIndexUtilTest {
new StreamKey(/* periodIndex= */ 0, /* groupIndex= */ 1, /* trackIndex= */ 2);
DownloadAction action1 =
DownloadAction.createDownloadAction(
"id",
TYPE_DASH,
Uri.parse("https://www.test.com/download1"),
asList(streamKey1),
@ -86,6 +88,7 @@ public class DownloadIndexUtilTest {
new byte[] {1, 2, 3, 4});
DownloadAction action2 =
DownloadAction.createDownloadAction(
"id",
TYPE_DASH,
Uri.parse("https://www.test.com/download2"),
asList(streamKey2),
@ -114,6 +117,7 @@ public class DownloadIndexUtilTest {
new StreamKey(/* periodIndex= */ 0, /* groupIndex= */ 1, /* trackIndex= */ 2);
DownloadAction action1 =
DownloadAction.createDownloadAction(
"id1",
TYPE_DASH,
Uri.parse("https://www.test.com/download1"),
asList(streamKey1),
@ -121,6 +125,7 @@ public class DownloadIndexUtilTest {
new byte[] {1, 2, 3, 4});
DownloadAction action2 =
DownloadAction.createDownloadAction(
"id2",
TYPE_DASH,
Uri.parse("https://www.test.com/download2"),
asList(streamKey2),

View file

@ -100,6 +100,7 @@ public class DownloadStateTest {
public void mergeAction_actionHaveDifferentData_downloadStateDataIsUpdated() {
DownloadAction downloadAction =
DownloadAction.createDownloadAction(
"id",
DownloadAction.TYPE_DASH,
testUri,
Collections.emptyList(),
@ -246,6 +247,7 @@ public class DownloadStateTest {
StreamKey[] keys1, StreamKey[] keys2, StreamKey[] expectedKeys) {
DownloadAction downloadAction =
DownloadAction.createDownloadAction(
"id",
DownloadAction.TYPE_DASH,
testUri,
Arrays.asList(keys2),
@ -290,6 +292,7 @@ public class DownloadStateTest {
private DownloadAction createDownloadAction() {
return DownloadAction.createDownloadAction(
"id",
DownloadAction.TYPE_DASH,
testUri,
Collections.emptyList(),

View file

@ -85,6 +85,7 @@ public class DashDownloaderTest {
Downloader downloader =
factory.createDownloader(
DownloadAction.createDownloadAction(
"id",
DownloadAction.TYPE_DASH,
Uri.parse("https://www.test.com/download"),
Collections.singletonList(new StreamKey(/* groupIndex= */ 0, /* trackIndex= */ 0)),

View file

@ -233,7 +233,12 @@ public class DownloadManagerDashTest {
Collections.addAll(keysList, keys);
DownloadAction action =
DownloadAction.createDownloadAction(
DownloadAction.TYPE_DASH, TEST_MPD_URI, keysList, /* customCacheKey= */ null, null);
TEST_ID,
DownloadAction.TYPE_DASH,
TEST_MPD_URI,
keysList,
/* customCacheKey= */ null,
null);
downloadManager.addDownload(action);
}

View file

@ -209,7 +209,12 @@ public class DownloadServiceDashTest {
Collections.addAll(keysList, keys);
DownloadAction action =
DownloadAction.createDownloadAction(
DownloadAction.TYPE_DASH, TEST_MPD_URI, keysList, /* customCacheKey= */ null, null);
TEST_ID,
DownloadAction.TYPE_DASH,
TEST_MPD_URI,
keysList,
/* customCacheKey= */ null,
null);
dummyMainThread.runOnMainThread(
() -> {
Intent startIntent =

View file

@ -102,6 +102,7 @@ public class HlsDownloaderTest {
Downloader downloader =
factory.createDownloader(
DownloadAction.createDownloadAction(
"id",
DownloadAction.TYPE_HLS,
Uri.parse("https://www.test.com/download"),
Collections.singletonList(new StreamKey(/* groupIndex= */ 0, /* trackIndex= */ 0)),

View file

@ -45,6 +45,7 @@ public final class SsDownloaderTest {
Downloader downloader =
factory.createDownloader(
DownloadAction.createDownloadAction(
"id",
DownloadAction.TYPE_SS,
Uri.parse("https://www.test.com/download"),
Collections.singletonList(new StreamKey(/* groupIndex= */ 0, /* trackIndex= */ 0)),