Start testing with a real server response

And fix a couple things. We should now be able to fetch content
from the production server.

- Set app_type back to TEST_APP
- I've replaced the old test feed query response with a new one
  from the new server. I've hand-edited the response to remove
  content that might be copyrighted, but it's unchanged other
  than that.
- Use our proto-printers to simplify the
  stream_model_update_request_unittest
- Add more .sh scripts to translate protos, making them all
  pipe stdin/stdout.

Bug: 1044139
Change-Id: Ib0053471ff732b3f4d6fe6ef653c17c41769ce62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2154811Reviewed-by: Ian Wells <iwells@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762066}

components/feed/core/proto/v2/wire/stream_structure.proto

@@ -24,12 +24,6 @@ message Cluster {
 // inside or outside a card. Actual data on what to display will be sent on an
 // extension.
 message Content {
-  enum Type {
-    UNKNOWN_CONTENT = 0;
-    XSURFACE = 1;
-  }
-  optional Type type = 1;
-
   optional bool is_ad = 3;
 
   optional XSurfaceContent xsurface_content = 1000;

components/feed/core/v2/feed_stream_unittest.cc

@@ -8,7 +8,10 @@
 #include <string>
 #include <utility>
 
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/optional.h"
+#include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/test/bind_test_util.h"
@@ -231,7 +234,12 @@ class TestFeedNetwork : public FeedNetwork {
     query_request_sent = request;
     QueryRequestResult result;
     result.status_code = 200;
-    result.response_body = std::make_unique<feedwire::Response>();
+    if (injected_response_) {
+      result.response_body = std::make_unique<feedwire::Response>(
+          std::move(injected_response_.value()));
+    } else {
+      result.response_body = std::make_unique<feedwire::Response>();
+    }
     base::SequencedTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::BindOnce(std::move(callback), std::move(result)));
   }
@@ -242,8 +250,25 @@ class TestFeedNetwork : public FeedNetwork {
   }
   void CancelRequests() override { NOTIMPLEMENTED(); }
 
+  void InjectRealResponse() {
+    base::FilePath response_file_path;
+    CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &response_file_path));
+    response_file_path = response_file_path.AppendASCII(
+        "components/test/data/feed/response.binarypb");
+    std::string response_data;
+    CHECK(base::ReadFileToString(response_file_path, &response_data));
+
+    feedwire::Response response;
+    CHECK(response.ParseFromString(response_data));
+
+    injected_response_ = response;
+  }
+
   base::Optional<feedwire::Request> query_request_sent;
   int send_query_call_count = 0;
+
+ private:
+  base::Optional<feedwire::Response> injected_response_;
 };
 
 // Forwards to |FeedStream::WireResponseTranslator| unless a response is
@@ -1062,5 +1087,13 @@ TEST_F(FeedStreamTest, LoadMoreBeforeLoad) {
   EXPECT_EQ(base::Optional<bool>(false), callback.GetResult());
 }
 
+TEST_F(FeedStreamTest, ReadNetworkResponse) {
+  network_.InjectRealResponse();
+  TestSurface surface(stream_.get());
+  WaitForIdleTaskQueue();
+
+  ASSERT_EQ("loading -> 10 slices", surface.DescribeUpdates());
+}
+
 }  // namespace
 }  // namespace feed

components/feed/core/v2/proto_util.cc

@@ -167,7 +167,7 @@ feedwire::ClientInfo CreateClientInfo(const ChromeInfo& chrome_info) {
 #elif defined(OS_IOS)
   client_info.set_platform_type(feedwire::ClientInfo::IOS);
 #endif
-  client_info.set_app_type(feedwire::ClientInfo::CHROME);
+  client_info.set_app_type(feedwire::ClientInfo::TEST_APP);
   *client_info.mutable_platform_version() = GetPlatformVersionMessage();
   *client_info.mutable_app_version() = GetAppVersionMessage(chrome_info);
   return client_info;

components/feed/core/v2/proto_util_unittest.cc

@@ -18,8 +18,8 @@ TEST(ProtoUtilTest, CreateClientInfo) {
   chrome_info.channel = version_info::Channel::STABLE;
 
   feedwire::ClientInfo result = CreateClientInfo(chrome_info);
-
-  EXPECT_EQ(feedwire::ClientInfo::CHROME, result.app_type());
+  // TODO(harringtond): change back to CHROME when it is supported.
+  EXPECT_EQ(feedwire::ClientInfo::TEST_APP, result.app_type());
   EXPECT_EQ(feedwire::Version::RELEASE, result.app_version().build_type());
   EXPECT_EQ(1, result.app_version().major());
   EXPECT_EQ(2, result.app_version().minor());

components/feed/core/v2/stream_model.cc

@@ -8,6 +8,7 @@
 #include <sstream>
 #include <utility>
 
+#include "base/json/string_escape.h"
 #include "base/logging.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
@@ -254,7 +255,9 @@ std::string StreamModel::DumpStateForTesting() {
   ss << "next_page_token='" << GetNextPageToken() << "'\n";
   ss << "consistency_token='" << GetConsistencyToken() << "'\n";
   for (auto& entry : shared_states_) {
-    ss << "shared_state[" << entry.first << "]='" << entry.second.data << "'\n";
+    ss << "shared_state[" << entry.first
+       << "]=" << base::GetQuotedJSONString(entry.second.data.substr(0, 100))
+       << "\n";
   }
   ss << GetFinalFeatureTree()->DumpStateForTesting();
   ss << "}StreamModel\n";

components/feed/core/v2/stream_model_update_request.cc

@@ -70,7 +70,7 @@ bool TranslateFeature(feedwire::Feature* feature,
   if (type == feedstore::StreamStructure::CONTENT) {
     feedwire::Content* wire_content = feature->mutable_content_extension();
 
-    if (wire_content->type() != feedwire::Content::XSURFACE)
+    if (!wire_content->has_xsurface_content())
       return false;
 
     // TODO(iwells): We still need score, availability_time_seconds,

components/feed/core/v2/test/proto_printer.cc

@@ -8,8 +8,8 @@
 #include <type_traits>
 #include "base/json/string_escape.h"
 #include "base/logging.h"
-
 #include "components/feed/core/proto/v2/wire/content_id.pb.h"
+#include "components/feed/core/v2/stream_model_update_request.h"
 
 namespace feed {
 namespace {}  // namespace
@@ -234,4 +234,23 @@ std::string ToTextProto(const feedstore::Record& v) {
   return TextProtoPrinter::ToString(v);
 }
 
+std::ostream& operator<<(std::ostream& os, const StreamModelUpdateRequest& v) {
+  os << "source: " << static_cast<int>(v.source) << '\n';
+  os << "stream_data: " << v.stream_data;
+  for (auto& content : v.content) {
+    os << "content: " << content;
+  }
+  for (auto& shared_state : v.shared_states) {
+    os << "shared_state: " << shared_state;
+  }
+  for (auto& stream_structure : v.stream_structures) {
+    os << "stream_structure: " << stream_structure;
+  }
+  os << "server_response_time: " << v.server_response_time << '\n';
+  os << "response_time: " << v.response_time << '\n';
+  os << "max_structure_sequence_number: " << v.max_structure_sequence_number
+     << '\n';
+  return os;
+}
+
 }  // namespace feed

components/feed/core/v2/test/proto_printer.h

@@ -14,6 +14,7 @@ namespace feedwire {
 class ContentId;
 }
 namespace feed {
+struct StreamModelUpdateRequest;
 
 std::string ToTextProto(const feedwire::ContentId& v);
 std::string ToTextProto(const feedstore::StreamData& v);
@@ -55,6 +56,8 @@ inline std::ostream& operator<<(std::ostream& os, const feedstore::Record& v) {
   return os << ToTextProto(v);
 }
 
+std::ostream& operator<<(std::ostream& os, const StreamModelUpdateRequest& v);
+
 }  // namespace feed
 
 #endif  // COMPONENTS_FEED_CORE_V2_TEST_PROTO_PRINTER_H_

components/feed/core/v2/tools/decode_feed_request.py

+#!/bin/bash
+# Copyright 2020 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+#
+# Usage: cat request_binary_file | ./decode_binary_feed_request.py
+
+CHROMIUM_SRC=$(realpath $(dirname $(readlink -f $0))/../../../../..)
+python3 \
+ $CHROMIUM_SRC/components/feed/core/v2/tools/textpb_to_binarypb.py \
+   --direction=reverse --chromium_path="$CHROMIUM_SRC" \
+   --message=feedwire.Request

components/feed/core/v2/tools/feed_response_to_textproto.sh → components/feed/core/v2/tools/decode_feed_response.sh

@@ -6,22 +6,14 @@
 # Converts a Feed HTTP response from binary to text using proto definitions from
 # Chromium.
 #
-# Usage: feed_response_to_textproto.sh <in.binarypb> <out.textproto>
-
-IN_FILE=$1
-OUT_FILE=$2
-TMP_FILE=/tmp/trimmedfeedresponse.binarypb
+# Usage: curl 'some url' | feed_response_to_textproto.sh
 
 CHROMIUM_SRC=$(realpath $(dirname $(readlink -f $0))/../../../../..)
 
 # Responses start with a varint length value that must be removed.
-cat $IN_FILE | python3 -c "import sys
+python3 -c "import sys
 while sys.stdin.buffer.read(1)[0]>127:
   pass
-sys.stdout.buffer.write(sys.stdin.buffer.read())" > $TMP_FILE
-
+sys.stdout.buffer.write(sys.stdin.buffer.read())" | \
 python3 $CHROMIUM_SRC/components/feed/core/v2/tools/textpb_to_binarypb.py \
-  --chromium_path=$CHROMIUM_SRC \
-  --output_file=$OUT_FILE \
-  --source_file=$TMP_FILE \
-  --direction=reverse
+  --chromium_path=$CHROMIUM_SRC --direction=reverse

components/feed/core/v2/tools/encode_feed_response.sh

+#!/bin/bash
+# Copyright 2020 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+#
+# Converts a Feed response textproto to a binary proto.
+#
+# Usage: cat some.textproto | feed_response_to_textproto.sh > result.binarypb
+
+CHROMIUM_SRC=$(realpath $(dirname $(readlink -f $0))/../../../../..)
+
+python3 $CHROMIUM_SRC/components/feed/core/v2/tools/textpb_to_binarypb.py \
+  --chromium_path=$CHROMIUM_SRC --direction=forward

components/feed/core/v2/tools/protoc_util.py

@@ -40,9 +40,12 @@ def get_protoc_common_args(root_dir, proto_path):
 
 def encode_proto(text, message_name, root_dir, proto_path):
   """Calls a command line to encode the text string and returns binary bytes."""
+  input_buffer = text
+  if isinstance(input_buffer, str):
+    input_buffer = text.encode()
   return run_command([protoc_path(root_dir), '--encode=' + message_name]
                      + get_protoc_common_args(root_dir, proto_path),
-                     text.encode())
+                     input_buffer)
 
 
 def decode_proto(data, message_name, root_dir, proto_path):

components/feed/core/v2/tools/textpb_to_binarypb.py

@@ -15,11 +15,12 @@ Usage example:
     --source_file /tmp/original.textpb
 """
 
+import base64
 import glob
 import os
 import protoc_util
 import subprocess
-import base64
+import sys
 import urllib.parse
 
 from absl import app
@@ -48,11 +49,14 @@ flags.DEFINE_string('direction', 'forward',
 
 COMPONENT_FEED_PROTO_PATH = 'components/feed/core/proto/v2'
 
-def text_to_binary():
-  with open(FLAGS.source_file, mode='r') as file:
-    value_text_proto = file.read()
+def read_input():
+  if FLAGS.source_file:
+    with open(FLAGS.source_file, mode='r') as file:
+      return file.read()
+  return sys.stdin.buffer.read()
 
-  encoded = protoc_util.encode_proto(value_text_proto, FLAGS.message,
+def text_to_binary():
+  encoded = protoc_util.encode_proto(read_input(), FLAGS.message,
                                      FLAGS.chromium_path,
                                      COMPONENT_FEED_PROTO_PATH)
 
@@ -64,13 +68,10 @@ def text_to_binary():
     with open(FLAGS.output_file, mode='wb') as file:
       file.write(encoded)
   else:
-    print(encoded)
+    sys.stdout.buffer.write(encoded)
 
 def binary_to_text():
-  with open(FLAGS.source_file, mode='rb') as file:
-    value_text_proto = file.read()
-
-  encoded = protoc_util.decode_proto(value_text_proto, FLAGS.message,
+  encoded = protoc_util.decode_proto(read_input(), FLAGS.message,
                                      FLAGS.chromium_path,
                                      COMPONENT_FEED_PROTO_PATH)
 
@@ -85,8 +86,6 @@ def main(argv):
     raise app.UsageError('Too many arguments. Unknown: ' + ' '.join(argv[1:]))
   if not FLAGS.chromium_path:
     raise app.UsageError('chromium_path flag must be set.')
-  if not FLAGS.source_file:
-    raise app.UsageError('source_file flag must be set.')
   if FLAGS.direction != 'forward' and FLAGS.direction != 'reverse':
     raise app.UsageError('direction must be forward or reverse')