Some more fiddling

Change-Id: Ie28d94d6ffa7bdbadd8e72a064538a3bf28dfc05

Author: samtstern

common/src/main/java/com/firebase/ui/common/BaseCachingSnapshotParser.java

@@ -0,0 +1,54 @@
+package com.firebase.ui.common;
+
+import android.support.annotation.RestrictTo;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Implementation of {@link BaseSnapshotParser} that caches results,
+ * so parsing a snapshot repeatedly is not expensive.
+ */
+@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
+public abstract class BaseCachingSnapshotParser<S, T> implements BaseSnapshotParser<S, T> {
+
+    private Map<String, T> mObjectCache = new HashMap<>();
+    private BaseSnapshotParser<S, T> mInnerParser;
+
+    // TODO(samstern): Can probably use this in RTDB module as well, CachingOSA is not necessary.
+    public BaseCachingSnapshotParser(BaseSnapshotParser<S, T> innerParser) {
+        mInnerParser = innerParser;
+    }
+
+    /**
+     * Get a unique identifier for a snapshot, should not depend on snapshot content.
+     */
+    public abstract String getId(S snapshot);
+
+    @Override
+    public T parseSnapshot(S snapshot) {
+        String id = getId(snapshot);
+        if (mObjectCache.containsKey(id)) {
+            return mObjectCache.get(id);
+        } else {
+            T object = mInnerParser.parseSnapshot(snapshot);
+            mObjectCache.put(id, object);
+            return object;
+        }
+    }
+
+    /**
+     * Clear all data in the cache.
+     */
+    public void clearData() {
+        mObjectCache.clear();
+    }
+
+    /**
+     * Invalidate the cache for a certain document ID.
+     */
+    public void invalidate(String id) {
+        mObjectCache.remove(id);
+    }
+
+}

common/src/main/java/com/firebase/ui/database/BaseObservableSnapshotArray.java renamed to common/src/main/java/com/firebase/ui/common/BaseObservableSnapshotArray.java

@@ -1,4 +1,4 @@
-package com.firebase.ui.database;
+package com.firebase.ui.common;
 
 import android.support.annotation.CallSuper;
 import android.support.annotation.NonNull;
@@ -8,18 +8,26 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
- * TODO
+ * Exposes a collection of {@link S} items in a database as a {@link List} of {@link E} objects.
+ * To observe the list attach a {@link L} listener.
+ *
  * @param <S> the snapshot class.
  * @param <L> the listener class.
  * @param <E> the model object class.
  */
 public abstract class BaseObservableSnapshotArray<S, L, E> extends AbstractList<S> {
 
     protected final List<L> mListeners = new CopyOnWriteArrayList<>();
-    protected final BaseSnapshotParser<S, E> mParser;
+    protected BaseSnapshotParser<S, E> mParser;
 
     private boolean mHasDataChanged = false;
 
+    /**
+     * Default constructor. Must set the {@link BaseSnapshotParser} before the first operation
+     * or an exception will be thrown.
+     */
+    public BaseObservableSnapshotArray() {}
+
     /**
      * Create an BaseObservableSnapshotArray with a custom {@link BaseSnapshotParser}.
      *
@@ -48,7 +56,7 @@ protected void onDestroy() {
     }
 
     /**
-     * TODO
+     * Attach a listener to this array.
      */
     @CallSuper
     public L addChangeEventListener(@NonNull L listener) {
@@ -59,7 +67,8 @@ public L addChangeEventListener(@NonNull L listener) {
     }
 
     /**
-     * TODO
+     * Remove a listener from the array. If no listeners remain, {@link #onDestroy()}
+     * will be called.
      */
     @CallSuper
     public void removeChangeEventListener(@NonNull L listener) {
@@ -72,7 +81,7 @@ public void removeChangeEventListener(@NonNull L listener) {
     }
 
     /**
-     * TODO
+     * Remove all listeners from the array.
      */
     @CallSuper
     public void removeAllListeners() {
@@ -81,20 +90,6 @@ public void removeAllListeners() {
         }
     }
 
-    protected abstract List<S> getSnapshots();
-
-    protected boolean hasDataChanged() {
-        return mHasDataChanged;
-    }
-
-    protected void setHasDataChanged(boolean hasDataChanged) {
-        mHasDataChanged = hasDataChanged;
-    }
-
-    protected BaseSnapshotParser<S, E> getSnapshotParser() {
-        return mParser;
-    }
-
     /**
      * @return true if the array is listening for change events from the Firebase
      * database, false otherwise
@@ -116,6 +111,10 @@ public final boolean isListening(L listener) {
      * initialized this will throw an unchecked exception.
      */
     public E getObject(int index) {
+        if (mParser == null) {
+            throw new IllegalStateException("getObject() called before snapshot parser set.");
+        }
+
         return mParser.parseSnapshot(get(index));
     }
 
@@ -128,4 +127,23 @@ public S get(int index) {
     public int size() {
         return getSnapshots().size();
     }
+
+    // TODO(samstern): Do we need this?
+    protected abstract List<S> getSnapshots();
+
+    protected boolean hasDataChanged() {
+        return mHasDataChanged;
+    }
+
+    protected void setHasDataChanged(boolean hasDataChanged) {
+        mHasDataChanged = hasDataChanged;
+    }
+
+    protected BaseSnapshotParser<S, E> getSnapshotParser() {
+        return mParser;
+    }
+
+    protected void setSnapshotParser(BaseSnapshotParser<S, E> parser) {
+        mParser = parser;
+    }
 }

common/src/main/java/com/firebase/ui/database/BaseSnapshotParser.java renamed to common/src/main/java/com/firebase/ui/common/BaseSnapshotParser.java

@@ -1,11 +1,14 @@
-package com.firebase.ui.database;
+package com.firebase.ui.common;
+
 
 public interface BaseSnapshotParser<S, T> {
+
     /**
      * This method parses the Snapshot into the requested type.
      *
      * @param snapshot the Snapshot to extract the model from
      * @return the model extracted from the DataSnapshot
      */
     T parseSnapshot(S snapshot);
+
 }

common/src/main/java/com/firebase/ui/database/Preconditions.java renamed to common/src/main/java/com/firebase/ui/common/Preconditions.java

@@ -1,12 +1,12 @@
-package com.firebase.ui.database;
+package com.firebase.ui.common;
 
 import android.support.annotation.RestrictTo;
 
 /**
  * Convenience class for checking argument conditions.
  */
 @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
-class Preconditions {
+public class Preconditions {
     public static <T> T checkNotNull(T o) {
         if (o == null) throw new IllegalArgumentException("Argument cannot be null.");
         return o;

database/src/main/java/com/firebase/ui/database/CachingObservableSnapshotArray.java

@@ -4,43 +4,35 @@
 
 import com.google.firebase.database.DataSnapshot;
 
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  * An extension of {@link ObservableSnapshotArray} that caches the result of {@link #getObject(int)}
  * so that repeated calls for the same key are not expensive (unless the underlying snapshot has
  * changed).
  */
 public abstract class CachingObservableSnapshotArray<T> extends ObservableSnapshotArray<T> {
-    private Map<String, T> mObjectCache = new HashMap<>();
+
+    private final CachingSnapshotParser<T> mCache;
 
     /**
      * @see ObservableSnapshotArray#ObservableSnapshotArray(Class)
      */
     public CachingObservableSnapshotArray(@NonNull Class<T> tClass) {
-        super(tClass);
+        this(new ClassSnapshotParser<>(tClass));
     }
 
     /**
      * @see ObservableSnapshotArray#ObservableSnapshotArray(SnapshotParser)
      */
     public CachingObservableSnapshotArray(@NonNull SnapshotParser<T> parser) {
-        super(parser);
+        super();
+
+        mCache = new CachingSnapshotParser<>(parser);
+        setSnapshotParser(mCache);
     }
 
     @Override
     public T getObject(int index) {
-        String key = get(index).getKey();
-
-        // Return from the cache if possible, otherwise populate the cache and return
-        if (mObjectCache.containsKey(key)) {
-            return mObjectCache.get(key);
-        } else {
-            T object = super.getObject(index);
-            mObjectCache.put(key, object);
-            return object;
-        }
+        return mCache.parseSnapshot(get(index));
     }
 
     @Override
@@ -53,20 +45,20 @@ protected void onDestroy() {
     @Deprecated
     protected void clearData() {
         getSnapshots().clear();
-        mObjectCache.clear();
+        mCache.clearData();
     }
 
     protected DataSnapshot removeData(int index) {
         DataSnapshot snapshot = getSnapshots().remove(index);
         if (snapshot != null) {
-            mObjectCache.remove(snapshot.getKey());
+            mCache.invalidate(snapshot.getKey());
         }
 
         return snapshot;
     }
 
     protected void updateData(int index, DataSnapshot snapshot) {
         getSnapshots().set(index, snapshot);
-        mObjectCache.remove(snapshot.getKey());
+        mCache.invalidate(snapshot.getKey());
     }
 }

database/src/main/java/com/firebase/ui/database/CachingSnapshotParser.java

@@ -0,0 +1,20 @@
+package com.firebase.ui.database;
+
+import com.firebase.ui.common.BaseCachingSnapshotParser;
+import com.firebase.ui.common.BaseSnapshotParser;
+import com.google.firebase.database.DataSnapshot;
+
+/**
+ * Implementation of {@link BaseCachingSnapshotParser} for {@link DataSnapshot}.
+ */
+public class CachingSnapshotParser<T> extends BaseCachingSnapshotParser<DataSnapshot, T> {
+
+    public CachingSnapshotParser(BaseSnapshotParser<DataSnapshot, T> innerParser) {
+        super(innerParser);
+    }
+
+    @Override
+    public String getId(DataSnapshot snapshot) {
+        return snapshot.getKey();
+    }
+}

database/src/main/java/com/firebase/ui/database/ChangeEventListener.java

@@ -5,6 +5,7 @@
 import com.google.firebase.database.DatabaseError;
 
 public interface ChangeEventListener {
+
     /**
      * The type of event received when a child has been updated.
      */

database/src/main/java/com/firebase/ui/database/ClassSnapshotParser.java

@@ -2,6 +2,7 @@
 
 import android.support.annotation.NonNull;
 
+import com.firebase.ui.common.Preconditions;
 import com.google.firebase.database.DataSnapshot;
 
 /**

database/src/main/java/com/firebase/ui/database/ObservableSnapshotArray.java

@@ -3,6 +3,7 @@
 import android.support.annotation.CallSuper;
 import android.support.annotation.NonNull;
 
+import com.firebase.ui.common.BaseObservableSnapshotArray;
 import com.google.firebase.database.DataSnapshot;
 import com.google.firebase.database.DatabaseError;
 
@@ -17,6 +18,13 @@
 public abstract class ObservableSnapshotArray<E>
         extends BaseObservableSnapshotArray<DataSnapshot, ChangeEventListener, E> {
 
+    /**
+     * Default constructor. Must set the snapshot parser before user.
+     */
+    public ObservableSnapshotArray() {
+        super();
+    }
+
     /**
      * Create an ObservableSnapshotArray where snapshots are parsed as objects of a particular
      * class.

database/src/main/java/com/firebase/ui/database/SnapshotParser.java

@@ -1,5 +1,6 @@
 package com.firebase.ui.database;
 
+import com.firebase.ui.common.BaseSnapshotParser;
 import com.google.firebase.database.DataSnapshot;
 
 public interface SnapshotParser<T> extends BaseSnapshotParser<DataSnapshot, T> {}