Merge #3237

3237: [snapshots] use user locale in timestamp output r=ricab a=sharder996

Co-authored-by: sharder996 <[email protected]>

Author: Chris Townsend

include/multipass/cli/format_utils.h

@@ -28,6 +28,8 @@
 #include <algorithm>
 #include <string>
 
+#define MP_FORMAT_UTILS multipass::FormatUtils::instance()
+
 namespace multipass
 {
 class Formatter;
@@ -58,6 +60,14 @@ static constexpr auto column_width =
         return std::max({get_width(*max_width) + col_buffer, header_width + col_buffer, minimum_width});
     };
 } // namespace format
+
+class FormatUtils : public Singleton<FormatUtils>
+{
+public:
+    FormatUtils(const Singleton<FormatUtils>::PrivatePass&) noexcept;
+
+    virtual std::string convert_to_user_locale(const google::protobuf::Timestamp& timestamp) const;
+};
 } // namespace multipass
 
 template <typename Container>

src/client/cli/formatter/csv_formatter.cpp

@@ -85,7 +85,7 @@ std::string generate_snapshot_details(const mp::InfoReply reply)
 
         fmt::format_to(std::back_inserter(buf),
                        ",{},{},{},\"{}\"\n",
-                       google::protobuf::util::TimeUtil::ToString(fundamentals.creation_timestamp()),
+                       MP_FORMAT_UTILS.convert_to_user_locale(fundamentals.creation_timestamp()),
                        fundamentals.parent(),
                        fmt::join(info.snapshot_info().children(), ";"),
                        fundamentals.comment());

src/client/cli/formatter/format_utils.cpp

@@ -15,14 +15,16 @@
  *
  */
 
+#include <multipass/cli/csv_formatter.h>
 #include <multipass/cli/format_utils.h>
 #include <multipass/cli/formatter.h>
-
-#include <multipass/cli/csv_formatter.h>
 #include <multipass/cli/json_formatter.h>
 #include <multipass/cli/table_formatter.h>
 #include <multipass/cli/yaml_formatter.h>
 
+#include <locale>
+#include <sstream>
+
 namespace mp = multipass;
 
 namespace
@@ -110,3 +112,19 @@ void mp::format::filter_aliases(google::protobuf::RepeatedPtrField<multipass::Fi
             aliases.DeleteSubrange(i, 1);
     }
 }
+
+mp::FormatUtils::FormatUtils(const Singleton<FormatUtils>::PrivatePass& pass) noexcept
+    : Singleton<FormatUtils>::Singleton{pass}
+{
+}
+
+std::string mp::FormatUtils::convert_to_user_locale(const google::protobuf::Timestamp& timestamp) const
+{
+    std::ostringstream oss;
+    oss.imbue(std::locale(""));
+
+    std::time_t t = google::protobuf::util::TimeUtil::TimestampToTimeT(timestamp);
+    oss << std::put_time(std::localtime(&t), "%c %Z");
+
+    return oss.str();
+}

src/client/cli/formatter/json_formatter.cpp

@@ -78,7 +78,7 @@ QJsonObject generate_snapshot_details(const mp::DetailedInfoItem& item)
 
     snapshot_info.insert(
         "created",
-        QString::fromStdString(google::protobuf::util::TimeUtil::ToString(fundamentals.creation_timestamp())));
+        QString::fromStdString(MP_FORMAT_UTILS.convert_to_user_locale(fundamentals.creation_timestamp())));
     snapshot_info.insert("parent", QString::fromStdString(fundamentals.parent()));
 
     QJsonArray children;

src/client/cli/formatter/table_formatter.cpp

@@ -88,7 +88,7 @@ std::string generate_snapshot_details(const mp::DetailedInfoItem& item)
     fmt::format_to(std::back_inserter(buf),
                    "{:<16}{}\n",
                    "Created:",
-                   google::protobuf::util::TimeUtil::ToString(fundamentals.creation_timestamp()));
+                   MP_FORMAT_UTILS.convert_to_user_locale(fundamentals.creation_timestamp()));
     fmt::format_to(std::back_inserter(buf),
                    "{:<16}{}\n",
                    "Parent:",

src/client/cli/formatter/yaml_formatter.cpp

@@ -78,7 +78,7 @@ YAML::Node generate_snapshot_details(const mp::DetailedInfoItem& item)
     }
     snapshot_node["mounts"] = mounts;
 
-    snapshot_node["created"] = google::protobuf::util::TimeUtil::ToString(fundamentals.creation_timestamp());
+    snapshot_node["created"] = MP_FORMAT_UTILS.convert_to_user_locale(fundamentals.creation_timestamp());
     snapshot_node["parent"] = fundamentals.parent().empty() ? YAML::Node() : YAML::Node(fundamentals.parent());
 
     snapshot_node["children"] = YAML::Node(YAML::NodeType::Sequence);

tests/mock_format_utils.h

@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef MULTIPASS_MOCK_FORMAT_UTILS_H
+#define MULTIPASS_MOCK_FORMAT_UTILS_H
+
+#include "mock_singleton_helpers.h"
+
+#include <multipass/cli/format_utils.h>
+
+namespace multipass::test
+{
+class MockFormatUtils : public FormatUtils
+{
+public:
+    using FormatUtils::FormatUtils;
+
+    MOCK_METHOD(std::string, convert_to_user_locale, (const google::protobuf::Timestamp&), (const, override));
+
+    MP_MOCK_SINGLETON_BOILERPLATE(MockFormatUtils, FormatUtils);
+};
+} // namespace multipass::test
+
+#endif // MULTIPASS_MOCK_FORMAT_UTILS_H

tests/test_output_formatter.cpp

@@ -16,6 +16,7 @@
  */
 
 #include "common.h"
+#include "mock_format_utils.h"
 #include "mock_settings.h"
 
 #include <multipass/cli/csv_formatter.h>
@@ -748,6 +749,11 @@ class BaseFormatterSuite : public testing::Test
         // The tests expected output are for the default C locale
         std::locale::global(std::locale("C"));
         EXPECT_CALL(mock_settings, get(Eq(mp::petenv_key))).WillRepeatedly(Return("pet"));
+
+        // Timestamps in tests need to be in a consistent locale
+        EXPECT_CALL(mock_format_utils, convert_to_user_locale(_)).WillRepeatedly([](const auto timestamp) {
+            return google::protobuf::util::TimeUtil::ToString(timestamp);
+        });
     }
 
     ~BaseFormatterSuite()
@@ -759,6 +765,9 @@ class BaseFormatterSuite : public testing::Test
     mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject<StrictMock>();
     mpt::MockSettings& mock_settings = *mock_settings_injection.first;
 
+    mpt::MockFormatUtils::GuardedMock mock_format_utils_injection = mpt::MockFormatUtils::inject<NiceMock>();
+    mpt::MockFormatUtils& mock_format_utils = *mock_format_utils_injection.first;
+
 private:
     std::locale saved_locale;
 };