ExternalConstantsBuilder - class to write override file for tests

This class uses the Builder pattern to write the config file loaded by
ExternalConstantsOverrider. "Builder" is not common in C++, compared to
Java, but here it seems like the easiest way to express our knowledge
about what fields are supported in this file type; it will create
reasonably terse integration test code and can be extended as new fields
are added to the set of overridable constants.

Bug: 1154901
Change-Id: Ia17280e5a6eaad8cd4e1d1f04f5fd0515ac2e3c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2631185
Commit-Queue: Adam Norberg <norberg@google.com>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844892}

chrome/updater/BUILD.gn

@@ -113,6 +113,8 @@ if (is_win || is_mac) {
       "crx_downloader_factory.h",
       "external_constants.cc",
       "external_constants.h",
+      "external_constants_builder.cc",
+      "external_constants_builder.h",
       "external_constants_impl.h",
       "external_constants_override.cc",
       "external_constants_override.h",
@@ -330,6 +332,7 @@ if (is_win || is_mac) {
     sources = [
       "app/app_server_unittest.cc",
       "enum_traits_unittest.cc",
+      "external_constants_builder_unittest.cc",
       "external_constants_override_unittest.cc",
       "external_constants_unittest.cc",
       "external_constants_unittest.h",

chrome/updater/external_constants_builder.cc

+// 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.
+
+#include "chrome/updater/external_constants_builder.h"
+
+#include <string>
+#include <vector>
+
+#include "base/json/json_file_value_serializer.h"
+#include "base/logging.h"
+#include "chrome/updater/constants.h"
+#include "chrome/updater/util.h"
+
+namespace updater {
+
+ExternalConstantsBuilder::~ExternalConstantsBuilder() {
+  LOG_IF(WARNING, !written_) << "An ExternalConstantsBuilder with "
+                             << overrides_.DictSize() << " entries is being "
+                             << "discarded without being written to a file.";
+}
+
+ExternalConstantsBuilder& ExternalConstantsBuilder::SetUpdateURL(
+    const std::vector<std::string>& urls) {
+  base::Value::ListStorage url_list;
+  url_list.reserve(urls.size());
+  for (const std::string& url_string : urls) {
+    url_list.push_back(base::Value(url_string));
+  }
+  overrides_.SetKey(kDevOverrideKeyUrl, base::Value(std::move(url_list)));
+  return *this;
+}
+
+ExternalConstantsBuilder& ExternalConstantsBuilder::ClearUpdateURL() {
+  overrides_.RemoveKey(kDevOverrideKeyUrl);
+  return *this;
+}
+
+ExternalConstantsBuilder& ExternalConstantsBuilder::SetUseCUP(bool use_cup) {
+  overrides_.SetBoolKey(kDevOverrideKeyUseCUP, use_cup);
+  return *this;
+}
+
+ExternalConstantsBuilder& ExternalConstantsBuilder::ClearUseCUP() {
+  overrides_.RemoveKey(kDevOverrideKeyUseCUP);
+  return *this;
+}
+
+ExternalConstantsBuilder& ExternalConstantsBuilder::SetInitialDelay(
+    int initial_delay) {
+  overrides_.SetIntKey(kDevOverrideKeyInitialDelay, initial_delay);
+  return *this;
+}
+
+ExternalConstantsBuilder& ExternalConstantsBuilder::ClearInitialDelay() {
+  overrides_.RemoveKey(kDevOverrideKeyInitialDelay);
+  return *this;
+}
+
+bool ExternalConstantsBuilder::Overwrite() {
+  base::FilePath base_path;
+  if (!GetBaseDirectory(&base_path)) {
+    LOG(ERROR) << "Can't find base directory; can't save constant overrides.";
+    return false;
+  }
+  const base::FilePath override_file_path =
+      base_path.AppendASCII(kDevOverrideFileName);
+  bool ok = JSONFileValueSerializer(override_file_path).Serialize(overrides_);
+  written_ = written_ || ok;
+  return ok;
+}
+
+}  // namespace updater

chrome/updater/external_constants_builder.h

+// 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.
+
+#ifndef CHROME_UPDATER_EXTERNAL_CONSTANTS_BUILDER_H_
+#define CHROME_UPDATER_EXTERNAL_CONSTANTS_BUILDER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/values.h"
+
+namespace updater {
+
+// ExternalConstantsBuilder uses the Builder design pattern to write a set of
+// overrides for default constant values to the file loaded by
+// ExternalConstantsOverrider. It is not thread-safe.
+//
+// When writing an overrides file, unset values (either because they were never
+// set or because they were cleared) are not included in the file, so the
+// "real" value would be used instead. An ExternalConstantsBuilder with
+// no values set would write an empty JSON object, which is a valid override
+// file that overrides nothing.
+//
+// If an ExternalConstantsBuilder is destroyed with no calls to Overwrite(),
+// it logs an error.
+class ExternalConstantsBuilder {
+ public:
+  ExternalConstantsBuilder() = default;
+  ~ExternalConstantsBuilder();
+
+  ExternalConstantsBuilder& SetUpdateURL(const std::vector<std::string>& urls);
+  ExternalConstantsBuilder& ClearUpdateURL();
+
+  ExternalConstantsBuilder& SetUseCUP(bool use_cup);
+  ExternalConstantsBuilder& ClearUseCUP();
+
+  ExternalConstantsBuilder& SetInitialDelay(int initial_delay);
+  ExternalConstantsBuilder& ClearInitialDelay();
+
+  // Write the external constants overrides file in the default location
+  // with the values that have been previously set, replacing any file
+  // previously there. The builder remains usable, does not forget its state,
+  // and subsequent calls to Overwrite will once again replace the file.
+  //
+  // Returns true on success, false on failure.
+  bool Overwrite();
+
+ private:
+  base::Value overrides_{base::Value::Type::DICTIONARY};
+  bool written_ = false;
+};
+
+}  // namespace updater
+
+#endif  // CHROME_UPDATER_EXTERNAL_CONSTANTS_BUILDER_H_

chrome/updater/external_constants_builder_unittest.cc

+// 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.
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/logging.h"
+#include "chrome/updater/constants.h"
+#include "chrome/updater/external_constants.h"
+#include "chrome/updater/external_constants_builder.h"
+#include "chrome/updater/external_constants_override.h"
+#include "chrome/updater/updater_branding.h"
+#include "chrome/updater/util.h"
+#include "url/gurl.h"
+
+namespace updater {
+
+namespace {
+
+void DeleteOverridesFile() {
+  base::FilePath target;
+  if (!GetBaseDirectory(&target)) {
+    LOG(ERROR) << "Could not get base directory to clean out overrides file.";
+  }
+  if (!base::DeleteFile(target.AppendASCII(kDevOverrideFileName))) {
+    // Note: base::DeleteFile returns `true` if there is no such file, which
+    // is what we want; "file already doesn't exist" is not an error here.
+    LOG(ERROR) << "Could not delete override file.";
+  }
+}
+
+}  // namespace
+
+class ExternalConstantsBuilderTests : public ::testing::Test {
+ protected:
+  void SetUp() override;
+  void TearDown() override;
+};
+
+void ExternalConstantsBuilderTests::SetUp() {
+  DeleteOverridesFile();
+}
+
+void ExternalConstantsBuilderTests::TearDown() {
+  DeleteOverridesFile();
+}
+
+TEST_F(ExternalConstantsBuilderTests, TestOverridingNothing) {
+  EXPECT_TRUE(ExternalConstantsBuilder().Overwrite());
+
+  std::unique_ptr<ExternalConstantsOverrider> verifier =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
+          CreateDefaultExternalConstantsForTesting());
+
+  EXPECT_TRUE(verifier->UseCUP());
+
+  std::vector<GURL> urls = verifier->UpdateURL();
+  ASSERT_EQ(urls.size(), 1ul);
+  EXPECT_EQ(urls[0], GURL(UPDATE_CHECK_URL));
+
+  EXPECT_EQ(verifier->InitialDelay(), kInitialDelay);
+}
+
+TEST_F(ExternalConstantsBuilderTests, TestOverridingEverything) {
+  ExternalConstantsBuilder builder;
+  builder.SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
+      .SetUseCUP(false)
+      .SetInitialDelay(123);
+  EXPECT_TRUE(builder.Overwrite());
+
+  std::unique_ptr<ExternalConstantsOverrider> verifier =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
+          CreateDefaultExternalConstantsForTesting());
+
+  EXPECT_FALSE(verifier->UseCUP());
+
+  std::vector<GURL> urls = verifier->UpdateURL();
+  ASSERT_EQ(urls.size(), 1ul);
+  EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+
+  EXPECT_EQ(verifier->InitialDelay(), 123);
+}
+
+TEST_F(ExternalConstantsBuilderTests, TestPartialOverrideWithMultipleURLs) {
+  ExternalConstantsBuilder builder;
+  EXPECT_TRUE(builder
+                  .SetUpdateURL(std::vector<std::string>{
+                      "https://www.google.com", "https://www.example.com"})
+                  .Overwrite());
+
+  std::unique_ptr<ExternalConstantsOverrider> verifier =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
+          CreateDefaultExternalConstantsForTesting());
+
+  EXPECT_TRUE(verifier->UseCUP());
+
+  std::vector<GURL> urls = verifier->UpdateURL();
+  ASSERT_EQ(urls.size(), 2ul);
+  EXPECT_EQ(urls[0], GURL("https://www.google.com"));
+  EXPECT_EQ(urls[1], GURL("https://www.example.com"));
+
+  EXPECT_EQ(verifier->InitialDelay(), kInitialDelay);
+}
+
+TEST_F(ExternalConstantsBuilderTests, TestClearedEverything) {
+  ExternalConstantsBuilder builder;
+  EXPECT_TRUE(builder
+                  .SetUpdateURL(std::vector<std::string>{
+                      "https://www.google.com", "https://www.example.com"})
+                  .SetUseCUP(false)
+                  .SetInitialDelay(123)
+                  .ClearUpdateURL()
+                  .ClearUseCUP()
+                  .ClearInitialDelay()
+                  .Overwrite());
+
+  std::unique_ptr<ExternalConstantsOverrider> verifier =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
+          CreateDefaultExternalConstantsForTesting());
+  EXPECT_TRUE(verifier->UseCUP());
+
+  std::vector<GURL> urls = verifier->UpdateURL();
+  ASSERT_EQ(urls.size(), 1ul);
+  EXPECT_EQ(urls[0], GURL(UPDATE_CHECK_URL));
+
+  EXPECT_EQ(verifier->InitialDelay(), kInitialDelay);
+}
+
+TEST_F(ExternalConstantsBuilderTests, TestOverSet) {
+  EXPECT_TRUE(
+      ExternalConstantsBuilder()
+          .SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
+          .SetUseCUP(true)
+          .SetInitialDelay(123)
+          .SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
+          .SetUseCUP(false)
+          .SetInitialDelay(937)
+          .Overwrite());
+
+  // Only the second set of values should be observed.
+  std::unique_ptr<ExternalConstantsOverrider> verifier =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
+          CreateDefaultExternalConstantsForTesting());
+  EXPECT_FALSE(verifier->UseCUP());
+
+  std::vector<GURL> urls = verifier->UpdateURL();
+  ASSERT_EQ(urls.size(), 1ul);
+  EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+
+  EXPECT_EQ(verifier->InitialDelay(), 937);
+}
+
+TEST_F(ExternalConstantsBuilderTests, TestReuseBuilder) {
+  ExternalConstantsBuilder builder;
+  EXPECT_TRUE(
+      builder.SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
+          .SetUseCUP(false)
+          .SetInitialDelay(123)
+          .SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
+          .Overwrite());
+
+  std::unique_ptr<ExternalConstantsOverrider> verifier =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
+          CreateDefaultExternalConstantsForTesting());
+
+  EXPECT_FALSE(verifier->UseCUP());
+
+  std::vector<GURL> urls = verifier->UpdateURL();
+  ASSERT_EQ(urls.size(), 1ul);
+  EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+
+  EXPECT_EQ(verifier->InitialDelay(), 123);
+
+  // But now we can use the builder again:
+  EXPECT_TRUE(builder.SetInitialDelay(92).ClearUpdateURL().Overwrite());
+
+  // We need a new overrider to verify because it only loads once.
+  std::unique_ptr<ExternalConstantsOverrider> verifier2 =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
+          CreateDefaultExternalConstantsForTesting());
+
+  EXPECT_FALSE(verifier2->UseCUP());  // Not updated, value should be retained.
+
+  std::vector<GURL> urls2 = verifier2->UpdateURL();
+  ASSERT_EQ(urls2.size(), 1ul);
+  EXPECT_EQ(urls2[0], GURL(UPDATE_CHECK_URL));  // Cleared; should be default.
+
+  EXPECT_EQ(verifier2->InitialDelay(), 92);  // Updated; update should be seen.
+}
+
+}  // namespace updater