Merge remote-tracking branch 'origin/master' into rsgowman/protobuf_cpp

Also "fixed" BadFieldValueTagWithOtherValidTagsPresent test by changing
'false' to 'true'. Details: Depending on the version of nanopb, nanopb
would explicitly encode 'false', which shouldn't be done in proto3. When
it's explicitly encoded, the test worked properly. But when it was
(properly) dropped, the invalid tag is the only field that's actually
encoded, thus violating the assumptions of the test, leading to a test
failure. s/false/true fixes it, as now the boolean_value field is
(properly) encoded regardless of version.

1 files changed, 40 insertions, 61 deletions

diff --git a/Firestore/core/include/firebase/firestore/document_reference.h b/Firestore/core/include/firebase/firestore/document_reference.h
index d295188..9385ed3 100644
--- a/Firestore/core/include/firebase/firestore/document_reference.h
+++ b/Firestore/core/include/firebase/firestore/document_reference.h
@@ -29,6 +29,17 @@
 #include <functional>
 #endif
 
+#include "firebase/app.h"
+#include "firebase/firestore/collection_reference.h"
+#include "firebase/firestore/document_snapshot.h"
+#include "firebase/firestore/event_listener.h"
+#include "firebase/firestore/field_value.h"
+#include "firebase/firestore/firestore.h"
+#include "firebase/firestore/firestore_errors.h"
+#include "firebase/firestore/listener_registration.h"
+#include "firebase/firestore/set_options.h"
+#include "firebase/future.h"
+
 // TODO(rsgowman): Note that RTDB uses:
 //   #if defined(FIREBASE_USE_MOVE_OPERATORS) || defined(DOXYGEN
 // to protect move operators from older compilers. But all our supported
@@ -36,34 +47,12 @@
 // here so we don't forget to mention this during the API review, and should be
 // removed once this note has migrated to the API review doc.
 
-// TODO(rsgowman): replace these forward decls with appropriate includes (once
-// they exist)
-namespace firebase {
-class App;
-template <typename T>
-class Future;
-}  // namespace firebase
-
 namespace firebase {
 namespace firestore {
 
-// TODO(rsgowman): replace these forward decls with appropriate includes (once
-// they exist)
-class FieldValue;
-class DocumentSnapshot;
+class DocumentReferenceInternal;
 class Firestore;
-class Error;
-template <typename T>
-class EventListener;
-class ListenerRegistration;
-class CollectionReference;
-class DocumentListenOptions;
-// TODO(rsgowman): not quite a forward decl, but required to make the default
-// parameter to Set() "compile".
-class SetOptions {
- public:
-  SetOptions();
-};
+class FirestoreInternal;
 
 // TODO(rsgowman): move this into the FieldValue header
 #ifdef STLPORT
@@ -80,12 +69,19 @@ using MapFieldValue = std::unordered_map<std::string, FieldValue>;
  *
  * Create a DocumentReference via Firebase::Document(const string& path).
  *
+ * NOT thread-safe: an instance should not be used from multiple threads
+ *
  * Subclassing Note: Firestore classes are not meant to be subclassed except for
  * use in test mocks. Subclassing is not supported in production code and new
  * SDK releases may break code that does so.
  */
 class DocumentReference {
  public:
+  enum class MetadataChanges {
+    kExclude,
+    kInclude,
+  };
+
   /**
    * @brief Default constructor. This creates an invalid DocumentReference.
    * Attempting to perform any operations on this reference will fail (and cause
@@ -269,28 +265,15 @@ class DocumentReference {
    * this DocumentReference. (Ownership is not transferred; you are responsible
    * for making sure that listener is valid as long as this DocumentReference is
    * valid and the listener is registered.)
+   * @param[in] metadata_changes Indicates whether metadata-only changes (i.e.
+   * only DocumentSnapshot.getMetadata() changed) should trigger snapshot
+   * events.
    *
    * @return A registration object that can be used to remove the listener.
    */
   virtual ListenerRegistration AddSnapshotListener(
-      EventListener<DocumentSnapshot>* listener);
-
-  /**
-   * @brief Starts listening to the document referenced by this
-   * DocumentReference.
-   *
-   * @param[in] options The options to use for this listen.
-   * @param[in] listener The event listener that will be called with the
-   * snapshots, which must remain in memory until you remove the listener from
-   * this DocumentReference. (Ownership is not transferred; you are responsible
-   * for making sure that listener is valid as long as this DocumentReference is
-   * valid and the listener is registered.)
-   *
-   * @return A registration object that can be used to remove the listener.
-   */
-  virtual ListenerRegistration AddSnapshotListener(
-      const DocumentListenOptions& options,
-      EventListener<DocumentSnapshot>* listener);
+      EventListener<DocumentSnapshot>* listener,
+      MetadataChanges metadata_changes = MetadataChanges::kExclude);
 
 #if defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
   /**
@@ -299,6 +282,9 @@ class DocumentReference {
    *
    * @param[in] callback function or lambda to call. When this function is
    * called, exactly one of the parameters will be non-null.
+   * @param[in] metadata_changes Indicates whether metadata-only changes (i.e.
+   * only DocumentSnapshot.getMetadata() changed) should trigger snapshot
+   * events.
    *
    * @return A registration object that can be used to remove the listener.
    *
@@ -306,28 +292,21 @@ class DocumentReference {
    * std::function is not supported on STLPort.
    */
   virtual ListenerRegistration AddSnapshotListener(
-      std::function<void(const DocumentSnapshot*, const Error*)> callback);
-
-  /**
-   * @brief Starts listening to the document referenced by this
-   * DocumentReference.
-   *
-   * @param[in] options The options to use for this listen.
-   * @param[in] callback function or lambda to call. When this function is
-   * called, exactly one of the parameters will be non-null.
-   *
-   * @return A registration object that can be used to remove the listener.
-   *
-   * @note This method is not available when using STLPort on Android, as
-   * std::function is not supported on STLPort.
-   */
-  virtual ListenerRegistration AddSnapshotListener(
-      const DocumentListenOptions& options,
-      std::function<void(const DocumentSnapshot*, const Error*)> callback);
+      std::function<void(const DocumentSnapshot*, const Error*)> callback,
+      MetadataChanges metadata_changes = MetadataChanges::kExclude);
 #endif  // defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
+
+ protected:
+  explicit DocumentReference(DocumentReferenceInternal* internal);
+
+ private:
+  friend class FirestoreInternal;
+
+  // TODO(zxu123): investigate possibility to use std::unique_ptr or
+  // firebase::UniquePtr.
+  DocumentReferenceInternal* internal_ = nullptr;
 };
 
-// TODO(rsgowman): probably define and inline here.
 bool operator==(const DocumentReference& lhs, const DocumentReference& rhs);
 
 inline bool operator!=(const DocumentReference& lhs,