Make CachedContent serialize whole metadata

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=189166039

library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java

@@ -29,6 +29,25 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
       0, 0, 0, 0, 0, 0, 10, 0, // original_content_length
       (byte) 0xF6, (byte) 0xFB, 0x50, 0x41 // hashcode_of_CachedContent_array
   };
+
+  private final byte[] testIndexV2File = {
+      0, 0, 0, 2, // version
+      0, 0, 0, 0, // flags
+      0, 0, 0, 2, // number_of_CachedContent
+      0, 0, 0, 1, // cache_id
+      0, 5, 65, 66, 67, 68, 69, // cache_key "ABCDE"
+      0, 0, 0, 1, // metadata count
+      0, 7, 101, 120, 111, 95, 108, 101, 110, // "exo_len"
+      0, 0, 0, 8, // value length
+      0, 0, 0, 0, 0, 0, 0, 10, // original_content_length
+      0, 0, 0, 0, // cache_id
+      0, 5, 75, 76, 77, 78, 79, // cache_key "KLMNO"
+      0, 0, 0, 1, // metadata count
+      0, 7, 101, 120, 111, 95, 108, 101, 110, // "exo_len"
+      0, 0, 0, 8, // value length
+      0, 0, 0, 0, 0, 0, 10, 0, // original_content_length
+      (byte) 0x42, (byte) 0x4A, (byte) 0x4F, (byte) 0x6F // hashcode_of_CachedContent_array
+  };
   private CachedContentIndex index;
   private File cacheDir;
 
@@ -51,8 +70,7 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
     final String key3 = "key3";
 
     // Add two CachedContents with add methods
-    CachedContent cachedContent1 = new CachedContent(5, key1);
-    index.addNew(cachedContent1);
+    CachedContent cachedContent1 = index.getOrAdd(key1);
     CachedContent cachedContent2 = index.getOrAdd(key2);
     assertThat(cachedContent1.id != cachedContent2.id).isTrue();
 
@@ -88,7 +106,7 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
     assertThat(cacheSpanFile.exists()).isTrue();
 
     // test removeEmpty()
-    index.addNew(cachedContent2);
+    index.getOrAdd(key2);
     index.removeEmpty();
     assertThat(index.get(key1)).isEqualTo(cachedContent1);
     assertThat(index.get(key2)).isNull();
@@ -112,25 +130,33 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
     assertThat(index.getContentLength("KLMNO")).isEqualTo(2560);
   }
 
-  public void testStoreV1() throws Exception {
-    CachedContent cachedContent1 = new CachedContent(2, "KLMNO");
+  public void testLoadV2() throws Exception {
+    FileOutputStream fos = new FileOutputStream(new File(cacheDir, CachedContentIndex.FILE_NAME));
+    fos.write(testIndexV2File);
+    fos.close();
+
+    index.load();
+    assertThat(index.getAll()).hasSize(2);
+    assertThat(index.assignIdForKey("ABCDE")).isEqualTo(1);
+    assertThat(index.getContentLength("ABCDE")).isEqualTo(10);
+    assertThat(index.assignIdForKey("KLMNO")).isEqualTo(0);
+    assertThat(index.getContentLength("KLMNO")).isEqualTo(2560);
+  }
+
+  public void testStore() throws Exception {
+    CachedContent cachedContent1 = index.getOrAdd("KLMNO");
     cachedContent1.setLength(2560);
-    index.addNew(cachedContent1);
-    CachedContent cachedContent2 = new CachedContent(5, "ABCDE");
+    CachedContent cachedContent2 = index.getOrAdd("ABCDE");
     cachedContent2.setLength(10);
-    index.addNew(cachedContent2);
 
     index.store();
 
-    byte[] buffer = new byte[testIndexV1File.length];
     FileInputStream fos = new FileInputStream(new File(cacheDir, CachedContentIndex.FILE_NAME));
-    assertThat(fos.read(buffer)).isEqualTo(testIndexV1File.length);
-    assertThat(fos.read()).isEqualTo(-1);
-    fos.close();
+    byte[] buffer = Util.toByteArray(fos);
 
     // TODO: The order of the CachedContent stored in index file isn't defined so this test may fail
     // on a different implementation of the underlying set
-    assertThat(buffer).isEqualTo(testIndexV1File);
+    assertThat(buffer).isEqualTo(testIndexV2File);
   }
 
   public void testAssignIdForKeyAndGetKeyForId() throws Exception {
@@ -214,14 +240,13 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
 
     // Test multiple store() calls
     CachedContentIndex index = new CachedContentIndex(cacheDir, key);
-    index.addNew(new CachedContent(15, "key3"));
+    index.getOrAdd("key3");
     index.store();
     assertStoredAndLoadedEqual(index, new CachedContentIndex(cacheDir, key));
   }
 
   public void testRemoveEmptyNotLockedCachedContent() throws Exception {
-    CachedContent cachedContent = new CachedContent(5, "key1");
-    index.addNew(cachedContent);
+    CachedContent cachedContent = index.getOrAdd("key1");
 
     index.maybeRemove(cachedContent.key);
 
@@ -229,8 +254,7 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
   }
 
   public void testCantRemoveNotEmptyCachedContent() throws Exception {
-    CachedContent cachedContent = new CachedContent(5, "key1");
-    index.addNew(cachedContent);
+    CachedContent cachedContent = index.getOrAdd("key1");
     File cacheSpanFile =
         SimpleCacheSpanTest.createCacheSpanFile(cacheDir, cachedContent.id, 10, 20, 30);
     SimpleCacheSpan span = SimpleCacheSpan.createCacheEntry(cacheSpanFile, index);
@@ -242,9 +266,8 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
   }
 
   public void testCantRemoveLockedCachedContent() throws Exception {
-    CachedContent cachedContent = new CachedContent(5, "key1");
+    CachedContent cachedContent = index.getOrAdd("key1");
     cachedContent.setLocked(true);
-    index.addNew(cachedContent);
 
     index.maybeRemove(cachedContent.key);
 
@@ -253,7 +276,7 @@ public class CachedContentIndexTest extends InstrumentationTestCase {
 
   private void assertStoredAndLoadedEqual(CachedContentIndex index, CachedContentIndex index2)
       throws IOException {
-    index.addNew(new CachedContent(5, "key1"));
+    index.getOrAdd("key1");
     index.getOrAdd("key2");
     index.store();
 

library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContent.java

@@ -28,8 +28,10 @@ import java.util.TreeSet;
  */
 /*package*/ final class CachedContent {
 
+  private static final int VERSION_METADATA_INTRODUCED = 2;
   private static final String EXOPLAYER_METADATA_NAME_PREFIX = "exo_";
   private static final String METADATA_NAME_LENGTH = EXOPLAYER_METADATA_NAME_PREFIX + "len";
+
   /** The cache file id that uniquely identifies the original stream. */
   public final int id;
   /** The cache key that uniquely identifies the original stream. */
@@ -44,15 +46,22 @@ import java.util.TreeSet;
   /**
    * Reads an instance from a {@link DataInputStream}.
    *
+   * @param version Version of the encoded data.
    * @param input Input stream containing values needed to initialize CachedContent instance.
    * @throws IOException If an error occurs during reading values.
    */
-  public static CachedContent readFromStream(DataInputStream input) throws IOException {
+  public static CachedContent readFromStream(int version, DataInputStream input)
+      throws IOException {
     int id = input.readInt();
     String key = input.readUTF();
-    long length = input.readLong();
-    CachedContent cachedContent = new CachedContent(id, key);
-    cachedContent.setLength(length);
+    CachedContent cachedContent;
+    if (version < VERSION_METADATA_INTRODUCED) {
+      cachedContent = new CachedContent(id, key);
+      long length = input.readLong();
+      cachedContent.setLength(length);
+    } else {
+      cachedContent = new CachedContent(id, key, DefaultContentMetadata.readFromStream(input));
+    }
     return cachedContent;
   }
 
@@ -63,9 +72,13 @@ import java.util.TreeSet;
    * @param key The cache stream key.
    */
   public CachedContent(int id, String key) {
+    this(id, key, new DefaultContentMetadata());
+  }
+
+  private CachedContent(int id, String key, DefaultContentMetadata metadata) {
     this.id = id;
     this.key = key;
-    this.metadata = new DefaultContentMetadata();
+    this.metadata = metadata;
     this.cachedSpans = new TreeSet<>();
   }
 
@@ -78,7 +91,7 @@ import java.util.TreeSet;
   public void writeToStream(DataOutputStream output) throws IOException {
     output.writeInt(id);
     output.writeUTF(key);
-    output.writeLong(getLength());
+    metadata.writeToStream(output);
   }
 
   /**
@@ -202,12 +215,19 @@ import java.util.TreeSet;
     return false;
   }
 
-  /** Calculates a hash code for the header of this {@code CachedContent}. */
-  public int headerHashCode() {
-    long length = getLength();
+  /**
+   * Calculates a hash code for the header of this {@code CachedContent} which is compatible with
+   * the index file with {@code version}.
+   */
+  public int headerHashCode(int version) {
     int result = id;
     result = 31 * result + key.hashCode();
-    result = 31 * result + (int) (length ^ (length >>> 32));
+    if (version < VERSION_METADATA_INTRODUCED) {
+      long length = getLength();
+      result = 31 * result + (int) (length ^ (length >>> 32));
+    } else {
+      result = 31 * result + metadata.hashCode();
+    }
     return result;
   }
 

library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndex.java

@@ -50,7 +50,7 @@ import javax.crypto.spec.SecretKeySpec;
 
   public static final String FILE_NAME = "cached_content_index.exi";
 
-  private static final int VERSION = 1;
+  private static final int VERSION = 2;
 
   private static final int FLAG_ENCRYPTED_INDEX = 1;
 
@@ -139,10 +139,7 @@ import javax.crypto.spec.SecretKeySpec;
    */
   public CachedContent getOrAdd(String key) {
     CachedContent cachedContent = keyToContent.get(key);
-    if (cachedContent == null) {
-      cachedContent = addNew(key, C.LENGTH_UNSET);
-    }
-    return cachedContent;
+    return cachedContent == null ? addNew(key) : cachedContent;
   }
 
   /** Returns a CachedContent instance with the given key or null if there isn't one. */
@@ -205,14 +202,10 @@ import javax.crypto.spec.SecretKeySpec;
    * one already with the given key.
    */
   public void setContentLength(String key, long length) {
-    CachedContent cachedContent = get(key);
-    if (cachedContent != null) {
-      if (cachedContent.getLength() != length) {
-        cachedContent.setLength(length);
-        changed = true;
-      }
-    } else {
-      addNew(key, length);
+    CachedContent cachedContent = getOrAdd(key);
+    if (cachedContent.getLength() != length) {
+      cachedContent.setLength(length);
+      changed = true;
     }
   }
 
@@ -231,8 +224,7 @@ import javax.crypto.spec.SecretKeySpec;
       InputStream inputStream = new BufferedInputStream(atomicFile.openRead());
       input = new DataInputStream(inputStream);
       int version = input.readInt();
-      if (version != VERSION) {
-        // Currently there is no other version
+      if (version < 0 || version > VERSION) {
         return false;
       }
 
@@ -257,9 +249,9 @@ import javax.crypto.spec.SecretKeySpec;
       int count = input.readInt();
       int hashCode = 0;
       for (int i = 0; i < count; i++) {
-        CachedContent cachedContent = CachedContent.readFromStream(input);
+        CachedContent cachedContent = CachedContent.readFromStream(version, input);
         add(cachedContent);
-        hashCode += cachedContent.headerHashCode();
+        hashCode += cachedContent.headerHashCode(version);
       }
       if (input.readInt() != hashCode) {
         return false;
@@ -310,7 +302,7 @@ import javax.crypto.spec.SecretKeySpec;
       int hashCode = 0;
       for (CachedContent cachedContent : keyToContent.values()) {
         cachedContent.writeToStream(output);
-        hashCode += cachedContent.headerHashCode();
+        hashCode += cachedContent.headerHashCode(VERSION);
       }
       output.writeInt(hashCode);
       atomicFile.endWrite(output);
@@ -324,23 +316,17 @@ import javax.crypto.spec.SecretKeySpec;
     }
   }
 
-  private void add(CachedContent cachedContent) {
-    keyToContent.put(cachedContent.key, cachedContent);
-    idToKey.put(cachedContent.id, cachedContent.key);
-  }
-
-  /** Adds the given CachedContent to the index. */
-  /*package*/ void addNew(CachedContent cachedContent) {
+  private CachedContent addNew(String key) {
+    int id = getNewId(idToKey);
+    CachedContent cachedContent = new CachedContent(id, key);
     add(cachedContent);
     changed = true;
+    return cachedContent;
   }
 
-  private CachedContent addNew(String key, long length) {
-    int id = getNewId(idToKey);
-    CachedContent cachedContent = new CachedContent(id, key);
-    cachedContent.setLength(length);
-    addNew(cachedContent);
-    return cachedContent;
+  private void add(CachedContent cachedContent) {
+    keyToContent.put(cachedContent.key, cachedContent);
+    idToKey.put(cachedContent.id, cachedContent.key);
   }
 
   private static Cipher getCipher() throws NoSuchPaddingException, NoSuchAlgorithmException {

library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/ContentMetadataMutations.java

@@ -17,11 +17,17 @@ package com.google.android.exoplayer2.upstream.cache;
 
 import com.google.android.exoplayer2.util.Assertions;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
-/** Defines multiple mutations on metadata value which are applied atomically. */
+/**
+ * Defines multiple mutations on metadata value which are applied atomically. This class isn't
+ * thread safe.
+ */
 public class ContentMetadataMutations {
 
   private final Map<String, Object> editedValues;
@@ -34,7 +40,8 @@ public class ContentMetadataMutations {
   }
 
   /**
-   * Adds a mutation to set a metadata value. Passing {@code null} as {@code value} isn't allowed.
+   * Adds a mutation to set a metadata value. Passing {@code null} as {@code name} or {@code value}
+   * isn't allowed.
    *
    * @param name The name of the metadata value.
    * @param value The value to be set.
@@ -45,7 +52,7 @@ public class ContentMetadataMutations {
   }
 
   /**
-   * Adds a mutation to set a metadata value.
+   * Adds a mutation to set a metadata value. Passing {@code null} as {@code name} isn't allowed.
    *
    * @param name The name of the metadata value.
    * @param value The value to be set.
@@ -56,15 +63,15 @@ public class ContentMetadataMutations {
   }
 
   /**
-   * Adds a mutation to set a metadata value. Passing {@code null} as {@code value} isn't allowed.
-   * {@code value} byte array shouldn't be modified after passed to this method.
+   * Adds a mutation to set a metadata value. Passing {@code null} as {@code name} or {@code value}
+   * isn't allowed.
    *
    * @param name The name of the metadata value.
    * @param value The value to be set.
    * @return This Editor instance, for convenience.
    */
   public ContentMetadataMutations set(String name, byte[] value) {
-    return checkAndSet(name, value);
+    return checkAndSet(name, Arrays.copyOf(value, value.length));
   }
 
   /**
@@ -79,24 +86,26 @@ public class ContentMetadataMutations {
     return this;
   }
 
-  /**
-   * Returns a list of names of metadata values to be removed. The returned array shouldn't be
-   * modified.
-   */
+  /** Returns a list of names of metadata values to be removed. */
   public List<String> getRemovedValues() {
-    return removedValues;
+    return Collections.unmodifiableList(new ArrayList<>(removedValues));
   }
 
-  /**
-   * Returns a map of metadata name, value pairs to be set. The returned map and the values in it
-   * shouldn't be modified.
-   */
+  /** Returns a map of metadata name, value pairs to be set. Values are copied.  */
   public Map<String, Object> getEditedValues() {
-    return editedValues;
+    HashMap<String, Object> hashMap = new HashMap<>(editedValues);
+    for (Entry<String, Object> entry : hashMap.entrySet()) {
+      Object value = entry.getValue();
+      if (value instanceof byte[]) {
+        byte[] bytes = (byte[]) value;
+        entry.setValue(Arrays.copyOf(bytes, bytes.length));
+      }
+    }
+    return Collections.unmodifiableMap(hashMap);
   }
 
   private ContentMetadataMutations checkAndSet(String name, Object value) {
-    editedValues.put(name, Assertions.checkNotNull(value));
+    editedValues.put(Assertions.checkNotNull(name), Assertions.checkNotNull(value));
     removedValues.remove(name);
     return this;
   }

library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadata.java

@@ -21,6 +21,7 @@ import java.io.DataOutputStream;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -31,6 +32,7 @@ import java.util.Map.Entry;
 public final class DefaultContentMetadata implements ContentMetadata {
 
   private static final int MAX_VALUE_LENGTH = 10 * 1024 * 1024;
+  private int hashCode;
 
   /**
    * Deserializes a {@link DefaultContentMetadata} from the given input stream.
@@ -93,7 +95,8 @@ public final class DefaultContentMetadata implements ContentMetadata {
   @Override
   public final byte[] get(String name, byte[] defaultValue) {
     if (metadata.containsKey(name)) {
-      return metadata.get(name);
+      byte[] bytes = metadata.get(name);
+      return Arrays.copyOf(bytes, bytes.length);
     } else {
       return defaultValue;
     }
@@ -124,6 +127,41 @@ public final class DefaultContentMetadata implements ContentMetadata {
     return metadata.containsKey(name);
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    DefaultContentMetadata that = (DefaultContentMetadata) o;
+    Map<String, byte[]> otherMetadata = that.metadata;
+    if (metadata.size() != otherMetadata.size()) {
+      return false;
+    }
+    for (Entry<String, byte[]> entry : metadata.entrySet()) {
+      byte[] value = entry.getValue();
+      byte[] otherValue = otherMetadata.get(entry.getKey());
+      if (!Arrays.equals(value, otherValue)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  @Override
+  public int hashCode() {
+    if (hashCode == 0) {
+      int result = 0;
+      for (Entry<String, byte[]> entry : metadata.entrySet()) {
+        result += entry.getKey().hashCode() ^ Arrays.hashCode(entry.getValue());
+      }
+      hashCode = result;
+    }
+    return hashCode;
+  }
+
   private static Map<String, byte[]> applyMutations(
       Map<String, byte[]> otherMetadata, ContentMetadataMutations mutations) {
     HashMap<String, byte[]> metadata = new HashMap<>(otherMetadata);
@@ -154,7 +192,7 @@ public final class DefaultContentMetadata implements ContentMetadata {
     } else if (value instanceof byte[]) {
       return (byte[]) value;
     } else {
-      throw new IllegalStateException();
+      throw new IllegalArgumentException();
     }
   }
 

library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadataTest.java

@@ -135,12 +135,10 @@ public class DefaultContentMetadataTest {
 
   @Test
   public void testSerializeDeserialize() throws Exception {
-    ContentMetadataMutations mutations = new ContentMetadataMutations();
-    mutations.set("metadata1 name", "value");
-    mutations.set("metadata2 name", 12345);
     byte[] metadata3 = {1, 2, 3};
-    mutations.set("metadata3 name", metadata3);
-    contentMetadata = new DefaultContentMetadata(contentMetadata, mutations);
+    contentMetadata =
+        createContentMetadata(
+            "metadata1 name", "value", "metadata2 name", 12345, "metadata3 name", metadata3);
 
     ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
     contentMetadata.writeToStream(new DataOutputStream(outputStream));
@@ -153,11 +151,48 @@ public class DefaultContentMetadataTest {
     assertThat(contentMetadata2.get("metadata3 name", new byte[] {})).isEqualTo(metadata3);
   }
 
-  private DefaultContentMetadata createContentMetadata(String... pairs) {
+  @Test
+  public void testEqualsStringValues() throws Exception {
+    DefaultContentMetadata metadata1 = createContentMetadata("metadata1", "value");
+    DefaultContentMetadata metadata2 = createContentMetadata("metadata1", "value");
+    assertThat(metadata1).isEqualTo(metadata2);
+  }
+
+  @Test
+  public void testEquals() throws Exception {
+    DefaultContentMetadata metadata1 =
+        createContentMetadata(
+            "metadata1", "value", "metadata2", 12345, "metadata3", new byte[] {1, 2, 3});
+    DefaultContentMetadata metadata2 =
+        createContentMetadata(
+            "metadata2", 12345, "metadata3", new byte[] {1, 2, 3}, "metadata1", "value");
+    assertThat(metadata1).isEqualTo(metadata2);
+    assertThat(metadata1.hashCode()).isEqualTo(metadata2.hashCode());
+  }
+
+  @Test
+  public void testNotEquals() throws Exception {
+    DefaultContentMetadata metadata1 = createContentMetadata("metadata1", new byte[] {1, 2, 3});
+    DefaultContentMetadata metadata2 = createContentMetadata("metadata1", new byte[] {3, 2, 1});
+    assertThat(metadata1).isNotEqualTo(metadata2);
+    assertThat(metadata1.hashCode()).isNotEqualTo(metadata2.hashCode());
+  }
+
+  private DefaultContentMetadata createContentMetadata(Object... pairs) {
     assertThat(pairs.length % 2).isEqualTo(0);
     ContentMetadataMutations mutations = new ContentMetadataMutations();
     for (int i = 0; i < pairs.length; i += 2) {
-      mutations.set(pairs[i], pairs[i + 1]);
+      String name = (String) pairs[i];
+      Object value = pairs[i + 1];
+      if (value instanceof String) {
+        mutations.set(name, (String) value);
+      } else if (value instanceof byte[]) {
+        mutations.set(name, (byte[]) value);
+      } else if (value instanceof Number) {
+        mutations.set(name, ((Number) value).longValue());
+      } else {
+        throw new IllegalArgumentException();
+      }
     }
     return new DefaultContentMetadata(new DefaultContentMetadata(), mutations);
   }