update_engine: Make InstallPlan's dump nicer

Its really hard to read anything out of the current InstallPlan's
logs. This CL makes it a bit more structured so it can be read easier.

Also added a few other properties of InstallPlan that were missing in
the Dump().

Added unittest for it too.

BUG=b:171829801
TEST=cros_workon_make --board reef --test update_engine

Change-Id: Iaa327e875877e9645ef8f0af875c280e11ee485d
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2558933
Tested-by: Amin Hassani <ahassani@chromium.org>
Auto-Submit: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/Android.bp b/Android.bp
index e5f8c31..a02b16f 100644
--- a/Android.bp
+++ b/Android.bp
@@ -701,6 +701,7 @@
         "payload_consumer/file_descriptor_utils_unittest.cc",
         "payload_consumer/file_writer_unittest.cc",
         "payload_consumer/filesystem_verifier_action_unittest.cc",
+        "payload_consumer/install_plan_unittest.cc",
         "payload_consumer/partition_update_generator_android_unittest.cc",
         "payload_consumer/postinstall_runner_action_unittest.cc",
         "payload_consumer/verity_writer_android_unittest.cc",
diff --git a/BUILD.gn b/BUILD.gn
index 582e515..ed85594 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -501,6 +501,7 @@
       "payload_consumer/file_descriptor_utils_unittest.cc",
       "payload_consumer/file_writer_unittest.cc",
       "payload_consumer/filesystem_verifier_action_unittest.cc",
+      "payload_consumer/install_plan_unittest.cc",
       "payload_consumer/postinstall_runner_action_unittest.cc",
       "payload_consumer/xz_extent_writer_unittest.cc",
       "payload_generator/ab_generator_unittest.cc",
diff --git a/aosp/update_attempter_android.cc b/aosp/update_attempter_android.cc
index d48293a..348f330 100644
--- a/aosp/update_attempter_android.cc
+++ b/aosp/update_attempter_android.cc
@@ -277,7 +277,6 @@
     }
   }
 
-  LOG(INFO) << "Using this install plan:";
   install_plan_.Dump();
 
   HttpFetcher* fetcher = nullptr;
diff --git a/cros/omaha_response_handler_action.cc b/cros/omaha_response_handler_action.cc
index 6a51c77..04cae3e 100644
--- a/cros/omaha_response_handler_action.cc
+++ b/cros/omaha_response_handler_action.cc
@@ -221,7 +221,6 @@
   TEST_AND_RETURN(HasOutputPipe());
   if (HasOutputPipe())
     SetOutputObject(install_plan_);
-  LOG(INFO) << "Using this install plan:";
   install_plan_.Dump();
 
   // Send the deadline data (if any) to Chrome through a file. This is a pretty
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc
index 71b2a49..2fa6a80 100644
--- a/cros/update_attempter.cc
+++ b/cros/update_attempter.cc
@@ -850,7 +850,6 @@
       SystemState::Get()->boot_control()));
   install_plan_->powerwash_required = powerwash;
 
-  LOG(INFO) << "Using this install plan:";
   install_plan_->Dump();
 
   auto install_plan_action =
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index 3aae663..c399d02 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -16,6 +16,9 @@
 
 #include "update_engine/payload_consumer/install_plan.h"
 
+#include <algorithm>
+#include <utility>
+
 #include <base/format_macros.h>
 #include <base/logging.h>
 #include <base/strings/string_number_conversions.h>
@@ -26,14 +29,29 @@
 #include "update_engine/payload_consumer/payload_constants.h"
 
 using std::string;
+using std::vector;
 
 namespace chromeos_update_engine {
 
+namespace {
 string PayloadUrlsToString(
     const decltype(InstallPlan::Payload::payload_urls)& payload_urls) {
   return "(" + base::JoinString(payload_urls, ",") + ")";
 }
 
+string VectorToString(const vector<std::pair<string, string>>& input,
+                      const string& separator) {
+  vector<string> vec;
+  std::transform(input.begin(),
+                 input.end(),
+                 std::back_inserter(vec),
+                 [](const auto& pair) {
+                   return base::JoinString({pair.first, pair.second}, ": ");
+                 });
+  return base::JoinString(vec, separator);
+}
+}  // namespace
+
 string InstallPayloadTypeToString(InstallPayloadType type) {
   switch (type) {
     case InstallPayloadType::kUnknown:
@@ -58,33 +76,10 @@
 }
 
 void InstallPlan::Dump() const {
-  string partitions_str;
-  for (const auto& partition : partitions) {
-    partitions_str +=
-        base::StringPrintf(", part: %s (source_size: %" PRIu64
-                           ", target_size %" PRIu64 ", postinst:%s)",
-                           partition.name.c_str(),
-                           partition.source_size,
-                           partition.target_size,
-                           utils::ToString(partition.run_postinstall).c_str());
-  }
-  string payloads_str;
-  for (const auto& payload : payloads) {
-    payloads_str += base::StringPrintf(
-        ", payload: (urls: %s, size: %" PRIu64 ", metadata_size: %" PRIu64
-        ", metadata signature: %s, hash: %s, payload type: %s"
-        ", fingerprint: %s, app id: %s)",
-        PayloadUrlsToString(payload.payload_urls).c_str(),
-        payload.size,
-        payload.metadata_size,
-        payload.metadata_signature.c_str(),
-        base::HexEncode(payload.hash.data(), payload.hash.size()).c_str(),
-        InstallPayloadTypeToString(payload.type).c_str(),
-        payload.fp.c_str(),
-        payload.app_id.c_str());
-  }
+  LOG(INFO) << "InstallPlan: \n" << ToString();
+}
 
-  string version_str = base::StringPrintf(", version: %s", version.c_str());
+string InstallPlan::ToString() const {
   string url_str = download_url;
   if (base::StartsWith(
           url_str, "fd://", base::CompareCase::INSENSITIVE_ASCII)) {
@@ -92,19 +87,65 @@
     url_str = utils::GetFilePath(fd);
   }
 
-  LOG(INFO) << "InstallPlan: " << (is_resume ? "resume" : "new_update")
-            << version_str
-            << ", source_slot: " << BootControlInterface::SlotName(source_slot)
-            << ", target_slot: " << BootControlInterface::SlotName(target_slot)
-            << ", initial url: " << url_str << payloads_str << partitions_str
-            << ", hash_checks_mandatory: "
-            << utils::ToString(hash_checks_mandatory)
-            << ", powerwash_required: " << utils::ToString(powerwash_required)
-            << ", switch_slot_on_reboot: "
-            << utils::ToString(switch_slot_on_reboot)
-            << ", run_post_install: " << utils::ToString(run_post_install)
-            << ", is_rollback: " << utils::ToString(is_rollback)
-            << ", write_verity: " << utils::ToString(write_verity);
+  vector<string> result_str;
+  result_str.emplace_back(VectorToString(
+      {
+          {"type", (is_resume ? "resume" : "new_update")},
+          {"version", version},
+          {"source_slot", BootControlInterface::SlotName(source_slot)},
+          {"target_slot", BootControlInterface::SlotName(target_slot)},
+          {"initial url", url_str},
+          {"hash_checks_mandatory", utils::ToString(hash_checks_mandatory)},
+          {"powerwash_required", utils::ToString(powerwash_required)},
+          {"switch_slot_on_reboot", utils::ToString(switch_slot_on_reboot)},
+          {"run_post_install", utils::ToString(run_post_install)},
+          {"is_rollback", utils::ToString(is_rollback)},
+          {"rollback_data_save_requested",
+           utils::ToString(rollback_data_save_requested)},
+          {"write_verity", utils::ToString(write_verity)},
+      },
+      "\n"));
+
+  for (const auto& partition : partitions) {
+    result_str.emplace_back(VectorToString(
+        {
+            {"Partition", partition.name},
+            {"source_size", base::NumberToString(partition.source_size)},
+            {"source_path", partition.source_path},
+            {"source_hash",
+             base::HexEncode(partition.source_hash.data(),
+                             partition.source_hash.size())},
+            {"target_size", base::NumberToString(partition.target_size)},
+            {"target_path", partition.target_path},
+            {"target_hash",
+             base::HexEncode(partition.target_hash.data(),
+                             partition.target_hash.size())},
+            {"run_postinstall", utils::ToString(partition.run_postinstall)},
+            {"postinstall_path", partition.postinstall_path},
+            {"filesystem_type", partition.filesystem_type},
+        },
+        "\n  "));
+  }
+
+  for (unsigned int i = 0; i < payloads.size(); ++i) {
+    const auto& payload = payloads[i];
+    result_str.emplace_back(VectorToString(
+        {
+            {"Payload", base::NumberToString(i)},
+            {"urls", PayloadUrlsToString(payload.payload_urls)},
+            {"size", base::NumberToString(payload.size)},
+            {"metadata_size", base::NumberToString(payload.metadata_size)},
+            {"metadata_signature", payload.metadata_signature},
+            {"hash", base::HexEncode(payload.hash.data(), payload.hash.size())},
+            {"type", InstallPayloadTypeToString(payload.type)},
+            {"fingerprint", payload.fp},
+            {"app_id", payload.app_id},
+            {"already_applied", utils::ToString(payload.already_applied)},
+        },
+        "\n  "));
+  }
+
+  return base::JoinString(result_str, "\n");
 }
 
 bool InstallPlan::LoadPartitionsFromSlots(BootControlInterface* boot_control) {
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 16e5674..2026635 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -45,6 +45,7 @@
   bool operator!=(const InstallPlan& that) const;
 
   void Dump() const;
+  std::string ToString() const;
 
   // Loads the |source_path| and |target_path| of all |partitions| based on the
   // |source_slot| and |target_slot| if available. Returns whether it succeeded
diff --git a/payload_consumer/install_plan_unittest.cc b/payload_consumer/install_plan_unittest.cc
new file mode 100644
index 0000000..2ca8d81
--- /dev/null
+++ b/payload_consumer/install_plan_unittest.cc
@@ -0,0 +1,82 @@
+//
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include <gtest/gtest.h>
+
+#include "update_engine/payload_consumer/install_plan.h"
+
+namespace chromeos_update_engine {
+
+TEST(InstallPlanTest, Dump) {
+  InstallPlan install_plan{
+      .download_url = "foo-download-url",
+      .version = "foo-version",
+      .payloads = {{
+          .payload_urls = {"url1", "url2"},
+          .metadata_signature = "foo-signature",
+          .hash = {0xb2, 0xb3},
+          .fp = "foo-fp",
+          .app_id = "foo-app-id",
+      }},
+      .source_slot = BootControlInterface::kInvalidSlot,
+      .target_slot = BootControlInterface::kInvalidSlot,
+      .partitions = {{
+          .name = "foo-partition_name",
+          .source_path = "foo-source-path",
+          .source_hash = {0xb1, 0xb2},
+          .target_path = "foo-target-path",
+          .target_hash = {0xb3, 0xb4},
+          .postinstall_path = "foo-path",
+          .filesystem_type = "foo-type",
+      }},
+  };
+
+  EXPECT_EQ(install_plan.ToString(),
+            R"(type: new_update
+version: foo-version
+source_slot: INVALID
+target_slot: INVALID
+initial url: foo-download-url
+hash_checks_mandatory: false
+powerwash_required: false
+switch_slot_on_reboot: true
+run_post_install: true
+is_rollback: false
+rollback_data_save_requested: false
+write_verity: true
+Partition: foo-partition_name
+  source_size: 0
+  source_path: foo-source-path
+  source_hash: B1B2
+  target_size: 0
+  target_path: foo-target-path
+  target_hash: B3B4
+  run_postinstall: false
+  postinstall_path: foo-path
+  filesystem_type: foo-type
+Payload: 0
+  urls: (url1,url2)
+  size: 0
+  metadata_size: 0
+  metadata_signature: foo-signature
+  hash: B2B3
+  type: unknown
+  fingerprint: foo-fp
+  app_id: foo-app-id
+  already_applied: false)");
+}
+
+}  // namespace chromeos_update_engine