Bundles Fix: OrderBy should be optional, not required (#7733)

* OrderBy should be optional, not required

* String format

Author: wu-hui

Firestore/core/src/bundle/bundle_serializer.cc

@@ -239,7 +239,9 @@ Filter DecodeUnaryFilter(JsonReader& reader, const json& filter) {
 
 OrderByList DecodeOrderBy(JsonReader& reader, const json& query) {
   OrderByList result;
-  for (const auto& order_by : reader.RequiredArray("orderBy", query)) {
+  std::vector<json> default_order_by;
+  for (const auto& order_by :
+       reader.OptionalArray("orderBy", query, default_order_by)) {
     FieldPath path =
         DecodeFieldReference(reader, reader.RequiredObject("field", order_by));
 
@@ -349,7 +351,7 @@ const std::vector<json>& JsonReader::RequiredArray(const char* name,
     }
   }
 
-  Fail("'%s' is missing or is not a string", name);
+  Fail("'%s' is missing or is not an array", name);
   return EmptyVector<json>();
 }
 

Firestore/core/test/unit/bundle/bundle_serializer_test.cc

@@ -21,6 +21,7 @@
 #include "Firestore/Protos/cpp/google/firestore/v1/document.pb.h"
 #include "Firestore/core/src/core/field_filter.h"
 #include "Firestore/core/src/core/query.h"
+#include "Firestore/core/src/core/target.h"
 #include "Firestore/core/src/local/local_serializer.h"
 #include "Firestore/core/src/model/database_id.h"
 #include "Firestore/core/src/nanopb/byte_string.h"
@@ -49,6 +50,7 @@ using ProtoMaybeDocument = ::firestore::client::MaybeDocument;
 using ProtoNamedQuery = ::firestore::NamedQuery;
 using ProtoValue = ::google::firestore::v1::Value;
 using core::Query;
+using core::Target;
 using local::LocalSerializer;
 using model::DatabaseId;
 using nanopb::ByteString;
@@ -924,6 +926,39 @@ TEST_F(BundleSerializerTest, DecodesOrderBys) {
   VerifyNamedQueryRoundtrip(original);
 }
 
+// By default, the queries used for testing in this file always have default
+// OrderBy ("__name__") generated. We need to explicitly remove that for this
+// test.
+TEST_F(BundleSerializerTest, DecodeMissingOrderBysWorks) {
+  // This is `NamedQueryToJson(testutil::Query("bundles/docs/colls"))` with
+  // orderBy field manually removed.
+  auto json_string = R"|(
+{
+  "name":"query-1",
+  "bundledQuery":{
+    "parent":"projects/p/databases/default/documents/bundles/docs",
+    "structuredQuery":{"from":[{"collectionId":"colls"}]}
+  },
+  "readTime":"2021-03-17T14:04:20.166729927Z"
+}
+)|";
+  JsonReader reader;
+  auto named_query =
+      bundle_serializer.DecodeNamedQuery(reader, Parse(json_string));
+
+  EXPECT_OK(reader.status());
+  EXPECT_EQ(named_query.query_name(), "query-1");
+
+  // Reconstruct a core::Query from the deserialized target, this is how
+  // eventually the named query is used.
+  const Target& target = named_query.bundled_query().target();
+  Query query(target.path(), target.collection_group(), target.filters(),
+              target.order_bys(), target.limit(),
+              named_query.bundled_query().limit_type(), target.start_at(),
+              target.end_at());
+  EXPECT_EQ(query.ToTarget(), testutil::Query("bundles/docs/colls").ToTarget());
+}
+
 TEST_F(BundleSerializerTest, DecodeInvalidOrderBysFails) {
   std::string json_string =
       NamedQueryJsonString(testutil::Query("colls")