Remove obsolete methods from nanopb::Reader

Author: rsgowman

Firestore/core/src/firebase/firestore/nanopb/reader.cc

@@ -16,55 +16,16 @@
 
 #include "Firestore/core/src/firebase/firestore/nanopb/reader.h"
 
-#include "Firestore/Protos/nanopb/google/firestore/v1beta1/document.nanopb.h"
-
 namespace firebase {
 namespace firestore {
 namespace nanopb {
 
 using firebase::firestore::util::Status;
-using std::int64_t;
-using std::uint64_t;
 
 Reader Reader::Wrap(const uint8_t* bytes, size_t length) {
   return Reader{pb_istream_from_buffer(bytes, length)};
 }
 
-uint32_t Reader::ReadTag() {
-  Tag tag;
-  if (!status_.ok()) return 0;
-
-  bool eof;
-  if (!pb_decode_tag(&stream_, &tag.wire_type, &tag.field_number, &eof)) {
-    Fail(PB_GET_ERROR(&stream_));
-    return 0;
-  }
-
-  // nanopb code always returns a false status when setting eof.
-  HARD_ASSERT(!eof, "nanopb set both ok status and eof to true");
-
-  last_tag_ = tag;
-  return tag.field_number;
-}
-
-bool Reader::RequireWireType(pb_wire_type_t wire_type) {
-  if (!status_.ok()) return false;
-  if (wire_type != last_tag_.wire_type) {
-    // TODO(rsgowman): We need to add much more context to the error messages so
-    // that we'll have a hope of debugging them when a customer reports these
-    // errors. Ideally:
-    // - last_tag_'s field_number and wire_type.
-    // - containing message type
-    // - anything else that we can possibly get our hands on.
-    // Here, and throughout nanopb/*.cc, remote/*.cc, local/*.cc.
-    Fail(
-        "Input proto bytes cannot be parsed (mismatch between the wiretype and "
-        "the field number (tag))");
-    return false;
-  }
-  return true;
-}
-
 void Reader::ReadNanopbMessage(const pb_field_t fields[], void* dest_struct) {
   if (!status_.ok()) return;
 
@@ -77,101 +38,6 @@ void Reader::FreeNanopbMessage(const pb_field_t fields[], void* dest_struct) {
   pb_release(fields, dest_struct);
 }
 
-/**
- * Note that (despite the return type) this works for bool, enum, int32, int64,
- * uint32 and uint64 proto field types.
- *
- * Note: This is not expected to be called directly, but rather only via the
- * other Decode* methods (i.e. DecodeBool, DecodeLong, etc)
- *
- * @return The decoded varint as a uint64_t.
- */
-uint64_t Reader::ReadVarint() {
-  RequireWireType(PB_WT_VARINT);
-  if (!status_.ok()) return 0;
-
-  uint64_t varint_value = 0;
-  if (!pb_decode_varint(&stream_, &varint_value)) {
-    Fail(PB_GET_ERROR(&stream_));
-  }
-  return varint_value;
-}
-
-void Reader::ReadNull() {
-  uint64_t varint = ReadVarint();
-  if (!status_.ok()) return;
-
-  if (varint != google_protobuf_NullValue_NULL_VALUE) {
-    Fail("Input proto bytes cannot be parsed (invalid null value)");
-  }
-}
-
-bool Reader::ReadBool() {
-  uint64_t varint = ReadVarint();
-  if (!status_.ok()) return false;
-
-  switch (varint) {
-    case 0:
-      return false;
-    case 1:
-      return true;
-    default:
-      Fail("Input proto bytes cannot be parsed (invalid bool value)");
-      return false;
-  }
-}
-
-int64_t Reader::ReadInteger() {
-  return ReadVarint();
-}
-
-std::string Reader::ReadString() {
-  RequireWireType(PB_WT_STRING);
-  if (!status_.ok()) return "";
-
-  pb_istream_t substream;
-  if (!pb_make_string_substream(&stream_, &substream)) {
-    Fail(PB_GET_ERROR(&stream_));
-    return "";
-  }
-
-  std::string result(substream.bytes_left, '\0');
-  if (!pb_read(&substream, reinterpret_cast<pb_byte_t*>(&result[0]),
-               substream.bytes_left)) {
-    Fail(PB_GET_ERROR(&substream));
-    pb_close_string_substream(&stream_, &substream);
-    return "";
-  }
-
-  // NB: future versions of nanopb read the remaining characters out of the
-  // substream (and return false if that fails) as an additional safety
-  // check within pb_close_string_substream. Unfortunately, that's not present
-  // in the current version (0.38).  We'll make a stronger assertion and check
-  // to make sure there *are* no remaining characters in the substream.
-  HARD_ASSERT(
-      substream.bytes_left == 0,
-      "Bytes remaining in substream after supposedly reading all of them.");
-
-  pb_close_string_substream(&stream_, &substream);
-
-  return result;
-}
-
-std::vector<uint8_t> Reader::ReadBytes() {
-  std::string bytes = ReadString();
-  if (!status_.ok()) return {};
-
-  return std::vector<uint8_t>(bytes.begin(), bytes.end());
-}
-
-void Reader::SkipUnknown() {
-  if (!status_.ok()) return;
-
-  if (!pb_skip_field(&stream_, last_tag_.wire_type)) {
-    Fail(PB_GET_ERROR(&stream_));
-  }
-}
-
 }  // namespace nanopb
 }  // namespace firestore
 }  // namespace firebase

Firestore/core/src/firebase/firestore/nanopb/reader.h

@@ -20,19 +20,9 @@
 #include <pb.h>
 #include <pb_decode.h>
 
-#include <cstdint>
-#include <functional>
-#include <memory>
-#include <string>
-#include <utility>
-#include <vector>
-
 #include "Firestore/core/include/firebase/firestore/firestore_errors.h"
-#include "Firestore/core/src/firebase/firestore/nanopb/tag.h"
-#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
 #include "Firestore/core/src/firebase/firestore/util/status.h"
 #include "absl/strings/string_view.h"
-#include "absl/types/optional.h"
 
 namespace firebase {
 namespace firestore {
@@ -59,25 +49,6 @@ class Reader {
    */
   static Reader Wrap(const uint8_t* bytes, size_t length);
 
-  /**
-   * Reads a message type from the input stream.
-   *
-   * This essentially wraps calls to nanopb's pb_decode_tag() method.
-   *
-   * In addition to returning the tag, this method also stores it. Subsequent
-   * calls to ReadX will use the stored last tag to verify that the type is
-   * correct (and will otherwise set the status of this Reader object to a
-   * non-ok value with the code set to FirestoreErrorCode::DataLoss).
-   *
-   * @return The field number of the tag. Technically, this differs slightly
-   * from the tag itself insomuch as it doesn't include the wire type.
-   */
-  uint32_t ReadTag();
-
-  const Tag& last_tag() const {
-    return last_tag_;
-  }
-
   /**
    * Reads a nanopb message from the input stream.
    *
@@ -102,83 +73,6 @@ class Reader {
    */
   void FreeNanopbMessage(const pb_field_t fields[], void* dest_struct);
 
-  void ReadNull();
-  bool ReadBool();
-  std::int64_t ReadInteger();
-
-  std::string ReadString();
-
-  std::vector<uint8_t> ReadBytes();
-
-  /**
-   * Reads a message and its length.
-   *
-   * Analog to Writer::WriteNestedMessage(). See that methods docs for further
-   * details.
-   *
-   * Call this method when reading a nested message. Provide a function to read
-   * the message itself. An overload exists to allow the function to return
-   * either an optional or a unique_ptr.
-   *
-   * @param read_message_fn Function to read the submessage. Note that this
-   * function should return {} (or nullptr/nullopt) on error.
-   * @return Empty (i.e. nullptr/nullopt) on failure, else the deserialized
-   * value.
-   */
-  template <typename T>
-  absl::optional<T> ReadNestedMessage(
-      const std::function<absl::optional<T>(Reader*)>& read_message_fn) {
-    return ReadNestedMessageImpl(read_message_fn);
-  }
-  template <typename T>
-  std::unique_ptr<T> ReadNestedMessage(
-      const std::function<std::unique_ptr<T>(Reader*)>& read_message_fn) {
-    return ReadNestedMessageImpl(read_message_fn);
-  }
-
-  template <typename T, typename C>
-  using ReadingMemberFunction = T (C::*)(Reader*) const;
-
-  /**
-   * Reads a message and its length.
-   *
-   * Identical to ReadNestedMessage(), except this additionally takes the
-   * serializer (either local or remote) as the first parameter, thus allowing
-   * non-static methods to be used as the read_message_member_fn.
-   */
-  template <typename T, typename C>
-  absl::optional<T> ReadNestedMessage(
-      const C& serializer,
-      ReadingMemberFunction<absl::optional<T>, C> read_message_member_fn) {
-    return ReadNestedMessageImpl(serializer, read_message_member_fn);
-  }
-  template <typename T, typename C>
-  std::unique_ptr<T> ReadNestedMessage(
-      const C& serializer,
-      ReadingMemberFunction<std::unique_ptr<T>, C> read_message_member_fn) {
-    return ReadNestedMessageImpl(serializer, read_message_member_fn);
-  }
-
-  /**
-   * Discards the bytes associated with the last read tag. (According to the
-   * proto spec, we must ignore unknown fields.)
-   *
-   * This method uses the last tag read via ReadTag to determine how many bytes
-   * should be discarded.
-   */
-  void SkipUnknown();
-
-  size_t bytes_left() const {
-    return stream_.bytes_left;
-  }
-
-  /**
-   * True if the stream still has bytes left, and the status is ok.
-   */
-  bool good() const {
-    return stream_.bytes_left && status_.ok();
-  }
-
   util::Status status() const {
     return status_;
   }
@@ -208,95 +102,11 @@ class Reader {
   explicit Reader(pb_istream_t stream) : stream_(stream) {
   }
 
-  /**
-   * Ensures the last read tag (set via ReadTag()) is of the specified type. If
-   * not, then Reader::status() will return a non-ok value (with the code set to
-   * FirestoreErrorCode::DataLoss).
-   *
-   * @return Convenience indicator for success. (If false, then status() will
-   * return a non-ok value.)
-   */
-  bool RequireWireType(pb_wire_type_t wire_type);
-
-  /**
-   * Reads a "varint" from the input stream.
-   *
-   * This essentially wraps calls to nanopb's pb_decode_varint() method.
-   *
-   * Note that (despite the return type) this works for bool, enum, int32,
-   * int64, uint32 and uint64 proto field types.
-   *
-   * Note: This is not expected to be called direclty, but rather only via the
-   * other Decode* methods (i.e. DecodeBool, DecodeLong, etc)
-   *
-   * @return The decoded varint as a uint64_t.
-   */
-  std::uint64_t ReadVarint();
-
-  template <typename T>
-  T ReadNestedMessageImpl(const std::function<T(Reader*)>& read_message_fn);
-
-  template <typename T, typename C>
-  T ReadNestedMessageImpl(const C& serializer,
-                          ReadingMemberFunction<T, C> read_message_member_fn);
-
   util::Status status_ = util::Status::OK();
 
   pb_istream_t stream_;
-
-  Tag last_tag_;
 };
 
-template <typename T>
-T Reader::ReadNestedMessageImpl(
-    const std::function<T(Reader*)>& read_message_fn) {
-  // Implementation note: This is roughly modeled on pb_decode_delimited,
-  // adjusted to account for the oneof in FieldValue.
-
-  RequireWireType(PB_WT_STRING);
-  if (!status_.ok()) return {};
-
-  pb_istream_t raw_substream;
-  if (!pb_make_string_substream(&stream_, &raw_substream)) {
-    status_ =
-        util::Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_));
-    return read_message_fn(this);
-  }
-  Reader substream(raw_substream);
-
-  // If this fails, we *won't* return right away so that we can cleanup the
-  // substream (although technically, that turns out not to matter; no resource
-  // leaks occur if we don't do this.)
-  // TODO(rsgowman): Consider RAII here. (Watch out for Reader class which also
-  // wraps streams.)
-  T message = read_message_fn(&substream);
-  status_ = substream.status();
-
-  // NB: future versions of nanopb read the remaining characters out of the
-  // substream (and return false if that fails) as an additional safety
-  // check within pb_close_string_substream. Unfortunately, that's not present
-  // in the current version (0.38).  We'll make a stronger assertion and check
-  // to make sure there *are* no remaining characters in the substream.
-  HARD_ASSERT(
-      substream.bytes_left() == 0,
-      "Bytes remaining in substream after supposedly reading all of them.");
-
-  pb_close_string_substream(&stream_, &substream.stream_);
-
-  return message;
-}
-
-template <typename T, typename C>
-T Reader::ReadNestedMessageImpl(
-    const C& serializer,
-    Reader::ReadingMemberFunction<T, C> read_message_member_fn) {
-  std::function<T(Reader*)> read_message_fn = [=](Reader* reader) {
-    return (serializer.*read_message_member_fn)(reader);
-  };
-
-  return ReadNestedMessageImpl(std::move(read_message_fn));
-}
-
 }  // namespace nanopb
 }  // namespace firestore
 }  // namespace firebase