From 328cdb7015136ff831a14f686d08f41f4e7421a1 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 8 Apr 2016 15:46:50 -0500 Subject: pull the timezone from timedate1 regardless of whether it appears on the bus before or after we startup --- include/datetime/dbus-shared.h | 42 ++++++ include/datetime/timezone-timedated.h | 6 +- src/timezone-timedated.cpp | 249 ++++++++++++++++++---------------- tests/test-timezone-timedated.cpp | 184 ++++++++++++++----------- 4 files changed, 283 insertions(+), 198 deletions(-) diff --git a/include/datetime/dbus-shared.h b/include/datetime/dbus-shared.h index 057ac6b..fd43ae8 100644 --- a/include/datetime/dbus-shared.h +++ b/include/datetime/dbus-shared.h @@ -28,5 +28,47 @@ #define BUS_POWERD_PATH "/com/canonical/powerd" #define BUS_POWERD_INTERFACE "com.canonical.powerd" +namespace ayatana { +namespace indicator { +namespace datetime { + +namespace Bus +{ + namespace Timedate1 + { + static constexpr char const * BUSNAME {"org.freedesktop.timedate1"}; + static constexpr char const * ADDR {"/org/freedesktop/timedate1"}; + static constexpr char const * IFACE {"org.freedesktop.timedate1"}; + + namespace Properties + { + static constexpr char const * TIMEZONE {"Timezone"}; + } + + namespace Methods + { + static constexpr char const * SET_TIMEZONE {"SetTimezone"}; + } + } + + namespace Properties + { + static constexpr char const * IFACE {"org.freedesktop.DBus.Properties"}; + + namespace Methods + { + static constexpr char const * GET {"Get"}; + } + + namespace Signals + { + static constexpr char const * PROPERTIES_CHANGED {"PropertiesChanged"}; + } + } +} + +} // namespace datetime +} // namespace indicator +} // namespace ayatana #endif /* INDICATOR_DATETIME_DBUS_SHARED_H */ diff --git a/include/datetime/timezone-timedated.h b/include/datetime/timezone-timedated.h index 336a148..0857706 100644 --- a/include/datetime/timezone-timedated.h +++ b/include/datetime/timezone-timedated.h @@ -20,8 +20,6 @@ #ifndef INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H #define INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H -#define DEFAULT_FILENAME "/etc/timezone" - #include // base class #include // std::string @@ -31,12 +29,12 @@ namespace indicator { namespace datetime { /** - * \brief A #Timezone that gets its information from monitoring a file, such as /etc/timezone + * \brief A #Timezone that gets its information from org.freedesktop.timedate1 */ class TimedatedTimezone: public Timezone { public: - TimedatedTimezone(std::string filename = DEFAULT_FILENAME); + TimedatedTimezone(); ~TimedatedTimezone(); private: diff --git a/src/timezone-timedated.cpp b/src/timezone-timedated.cpp index d38557b..9254612 100644 --- a/src/timezone-timedated.cpp +++ b/src/timezone-timedated.cpp @@ -17,6 +17,7 @@ * Charles Kerr */ +#include #include #include @@ -36,166 +37,186 @@ class TimedatedTimezone::Impl { public: - Impl(TimedatedTimezone& owner, std::string filename): - m_owner(owner), - m_filename(filename) + Impl(TimedatedTimezone& owner): + m_owner{owner}, + m_cancellable{g_cancellable_new()} { - g_debug("Filename is '%s'", filename.c_str()); - monitor_timezone_property(); + // set the fallback value + m_owner.timezone.set("Etc/Utc"); + + // watch for timedate1 on the bus + m_watcher_id = g_bus_watch_name( + G_BUS_TYPE_SYSTEM, + Bus::Timedate1::BUSNAME, + G_BUS_NAME_WATCHER_FLAGS_AUTO_START, + on_timedate1_appeared, + on_timedate1_vanished, + this, + nullptr); } ~Impl() { - clear(); - } + g_cancellable_cancel(m_cancellable); + g_clear_object(&m_cancellable); -private: + g_bus_unwatch_name(m_watcher_id); - void clear() - { - if (m_connection && m_signal_subscription_id) + if (m_connection != nullptr) { - g_dbus_connection_signal_unsubscribe (m_connection, m_signal_subscription_id); - m_signal_subscription_id = 0; - } + if (m_signal_subscription_id) + g_dbus_connection_signal_unsubscribe(m_connection, m_signal_subscription_id); - g_clear_object(&m_connection); + g_clear_object(&m_connection); + } } - static void on_properties_changed (GDBusConnection *connection G_GNUC_UNUSED, - const gchar *sender_name G_GNUC_UNUSED, - const gchar *object_path G_GNUC_UNUSED, - const gchar *interface_name G_GNUC_UNUSED, - const gchar *signal_name G_GNUC_UNUSED, - GVariant *parameters, - gpointer gself) - { - auto self = static_cast(gself); - const char *tz; - GVariant *changed_properties; - gchar **invalidated_properties; - - g_variant_get (parameters, "(s@a{sv}^as)", NULL, &changed_properties, &invalidated_properties); +private: - if (g_variant_lookup(changed_properties, "Timezone", "&s", &tz, NULL)) - self->notify_timezone(tz); - else if (g_strv_contains (invalidated_properties, "Timezone")) - self->notify_timezone(self->get_timezone_from_file(self->m_filename)); + static void on_timedate1_appeared(GDBusConnection * connection, + const gchar * /*name*/, + const gchar * /*name_owner*/, + gpointer gself) + { + static_cast(gself)->timedate1_appeared(connection); + } + void timedate1_appeared(GDBusConnection* connection) + { + // cache m_connection for later... + g_clear_object(&m_connection); + m_connection = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(connection))); - g_variant_unref (changed_properties); - g_strfreev (invalidated_properties); + ensure_propchange_subscription(); + ask_for_timezone(); } - void monitor_timezone_property() + void ensure_propchange_subscription() { - GError *err = nullptr; - - /* - * There is an unlikely race which happens if there is an activation - * and timezone change before our match rule is added. - */ - notify_timezone(get_timezone_from_file(m_filename)); - - /* - * Make sure the bus is around at least until we add the match rules, - * otherwise things (tests) are sad. - */ - m_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, - nullptr, - &err); - - if (err) + if (m_signal_subscription_id == 0) { - g_warning("Couldn't get bus connection: '%s'", err->message); - g_error_free(err); - return; - } - - m_signal_subscription_id = g_dbus_connection_signal_subscribe(m_connection, - "org.freedesktop.timedate1", - "org.freedesktop.DBus.Properties", - "PropertiesChanged", - "/org/freedesktop/timedate1", - NULL, G_DBUS_SIGNAL_FLAGS_NONE, + m_signal_subscription_id = g_dbus_connection_signal_subscribe( + m_connection, + Bus::Timedate1::IFACE, + Bus::Properties::IFACE, + Bus::Properties::Signals::PROPERTIES_CHANGED, + Bus::Timedate1::ADDR, + nullptr, + G_DBUS_SIGNAL_FLAGS_NONE, on_properties_changed, - this, nullptr); + this, + nullptr); + } } - void notify_timezone(std::string new_timezone) + static void on_timedate1_vanished(GDBusConnection * /*connection*/, + const gchar * name, + gpointer /*gself*/) { - g_debug("notify_timezone '%s'", new_timezone.c_str()); - if (!new_timezone.empty()) - m_owner.timezone.set(new_timezone); + g_debug("%s not present on system bus", name); } - std::string get_timezone_from_file(const std::string& filename) + static void on_properties_changed(GDBusConnection * /*connection*/, + const gchar * /*sender_name*/, + const gchar * /*object_path*/, + const gchar * /*interface_name*/, + const gchar * /*signal_name*/, + GVariant * parameters, + gpointer gself) { - GError * error; - GIOChannel * io_channel; - std::string ret; - - // read through filename line-by-line until we fine a nonempty non-comment line - error = nullptr; - io_channel = g_io_channel_new_file(filename.c_str(), "r", &error); - if (error == nullptr) - { - auto line = g_string_new(nullptr); - - while(ret.empty()) - { - const auto io_status = g_io_channel_read_line_string(io_channel, line, nullptr, &error); - if ((io_status == G_IO_STATUS_EOF) || (io_status == G_IO_STATUS_ERROR)) - break; - if (error != nullptr) - break; - - g_strstrip(line->str); + auto self = static_cast(gself); - if (!line->len) // skip empty lines - continue; + GVariant* changed_properties {}; + gchar** invalidated_properties {}; + g_variant_get(parameters, "(s@a{sv}^as)", NULL, &changed_properties, &invalidated_properties); - if (*line->str=='#') // skip comments - continue; + const char* tz {}; + if (g_variant_lookup(changed_properties, Bus::Timedate1::Properties::TIMEZONE, "&s", &tz, NULL)) + { + if (tz != nullptr) + self->set_timezone(tz); + else + g_warning("%s no timezone found", G_STRLOC); + } + else if (g_strv_contains(invalidated_properties, Bus::Timedate1::Properties::TIMEZONE)) + { + self->ask_for_timezone(); + } - ret = line->str; - } + g_variant_unref(changed_properties); + g_strfreev(invalidated_properties); + } - g_string_free(line, true); - } else - /* Default to UTC */ - ret = "Etc/Utc"; + void ask_for_timezone() + { + g_return_if_fail(m_connection != nullptr); + + g_dbus_connection_call( + m_connection, + Bus::Timedate1::BUSNAME, + Bus::Timedate1::ADDR, + Bus::Properties::IFACE, + Bus::Properties::Methods::GET, + g_variant_new("(ss)", Bus::Timedate1::IFACE, Bus::Timedate1::Properties::TIMEZONE), + G_VARIANT_TYPE("(v)"), + G_DBUS_CALL_FLAGS_NONE, + -1, + m_cancellable, + on_get_timezone_ready, + this); + } - if (io_channel != nullptr) + static void on_get_timezone_ready(GObject * connection, + GAsyncResult * res, + gpointer gself) + { + GError* error {}; + GVariant* v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(connection), res, &error); + if (v == nullptr) { - g_io_channel_shutdown(io_channel, false, nullptr); - g_io_channel_unref(io_channel); + if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + g_warning("%s Couldn't get timezone: %s", G_STRLOC, error->message); } - - if (error != nullptr) + else { - g_warning("%s Unable to read timezone file '%s': %s", G_STRLOC, filename.c_str(), error->message); - g_error_free(error); + GVariant* tzv {}; + g_variant_get(v, "(v)", &tzv); + const char* tz = g_variant_get_string(tzv, nullptr); + + if (tz != nullptr) + static_cast(gself)->set_timezone(tz); + else + g_warning("%s no timezone found", G_STRLOC); + + g_clear_pointer(&tzv, g_variant_unref); + g_clear_pointer(&v, g_variant_unref); } + } + + void set_timezone(const std::string& tz) + { + g_return_if_fail(!tz.empty()); - return ret; + g_debug("set timezone: '%s'", tz.c_str()); + m_owner.timezone.set(tz); } /*** **** ***/ - TimedatedTimezone & m_owner; - GDBusConnection *m_connection = nullptr; - unsigned long m_signal_subscription_id = 0; - std::string m_filename; + TimedatedTimezone& m_owner; + GCancellable* m_cancellable {}; + GDBusConnection* m_connection {}; + unsigned long m_signal_subscription_id {}; + unsigned int m_watcher_id {}; }; /*** **** ***/ -TimedatedTimezone::TimedatedTimezone(std::string filename): - impl(new Impl{*this, filename}) +TimedatedTimezone::TimedatedTimezone(): + impl{new Impl{*this}} { } diff --git a/tests/test-timezone-timedated.cpp b/tests/test-timezone-timedated.cpp index 2fdec12..b293e59 100644 --- a/tests/test-timezone-timedated.cpp +++ b/tests/test-timezone-timedated.cpp @@ -1,9 +1,5 @@ - /* - * Copyright 2013 Canonical Ltd. - * - * Authors: - * Charles Kerr + * Copyright © 2014-2016 Canonical Ltd. * * This program is free software: you can redistribute it and/or modify it * under the terms of the GNU General Public License version 3, as published @@ -16,127 +12,155 @@ * * You should have received a copy of the GNU General Public License along * with this program. If not, see . + * + * Authors: + * Charles Kerr + * Ted Gould */ -#include "timedated-fixture.h" +#include "glib-fixture.h" +#include #include -using ayatana::indicator::datetime::TimedatedTimezone; +#include -/*** -**** -***/ -#define TIMEZONE_FILE (SANDBOX"/timezone") +using namespace ayatana::indicator::datetime; + -class TimezoneFixture: public TimedateFixture +struct Timedate1Fixture: public GlibFixture { - private: +private: + + typedef GlibFixture super; - typedef TimedateFixture super; +protected: - protected: + GDBusConnection* m_bus {}; + GTestDBus* m_test_bus {}; void SetUp() override { - super::SetUp(); + super::SetUp(); + + // use a fake bus + m_test_bus = g_test_dbus_new(G_TEST_DBUS_NONE); + g_test_dbus_up(m_test_bus); + const char * address = g_test_dbus_get_bus_address(m_test_bus); + g_setenv("DBUS_SYSTEM_BUS_ADDRESS", address, true); + g_setenv("DBUS_SESSION_BUS_ADDRESS", address, true); + g_debug("test_dbus's address is %s", address); + + // get the bus + m_bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, nullptr); + g_dbus_connection_set_exit_on_close(m_bus, FALSE); + g_object_add_weak_pointer(G_OBJECT(m_bus), (gpointer*)&m_bus); } void TearDown() override { - super::TearDown(); + g_clear_object(&m_bus); + g_clear_object(&m_test_bus); + + super::TearDown(); } - public: + void start_timedate1(const std::string& tzid) + { + // start the store + auto json_parameters = g_strdup_printf("{\"Timezone\": \"%s\"}", tzid.c_str()); + const gchar* child_argv[] = { "python3", "-m", "dbusmock", "--template", "timedated", "--parameters", json_parameters, nullptr }; + GError* error = nullptr; + g_spawn_async(nullptr, (gchar**)child_argv, nullptr, G_SPAWN_SEARCH_PATH, nullptr, nullptr, nullptr, &error); + g_assert_no_error(error); + g_free(json_parameters); + + // wait for it to appear on the bus + wait_for_name_owned(m_bus, Bus::Timedate1::BUSNAME); + } + + bool wait_for_tzid(Timezone& tz, const std::string& tzid) + { + return wait_for([&tz, &tzid](){return tzid == tz.timezone.get();}); + } - /* convenience func to set the timezone file */ - void set_file(const std::string& text) + void set_timedate1_timezone(const std::string& tzid) { - g_debug("set_file %s %s", TIMEZONE_FILE, text.c_str()); - auto fp = fopen(TIMEZONE_FILE, "w+"); - fprintf(fp, "%s\n", text.c_str()); - fclose(fp); - sync(); + GError* error {}; + auto v = g_dbus_connection_call_sync( + m_bus, + Bus::Timedate1::BUSNAME, + Bus::Timedate1::ADDR, + Bus::Timedate1::IFACE, + Bus::Timedate1::Methods::SET_TIMEZONE, + g_variant_new("(sb)", tzid.c_str(), FALSE), + nullptr, + G_DBUS_CALL_FLAGS_NONE, + -1, + nullptr, + &error); + g_clear_pointer(&v, g_variant_unref); + g_assert_no_error(error); } }; -/** - * Test that timezone-timedated warns, but doesn't crash, if the timezone file doesn't exist - */ -TEST_F(TimezoneFixture, NoFile) -{ - remove(TIMEZONE_FILE); - ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS)); +/*** +**** +***/ - expectLogMessage(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, "*No such file or directory*"); - TimedatedTimezone tz(TIMEZONE_FILE); +TEST_F(Timedate1Fixture, HelloWorld) +{ } /** - * Test that timezone-timedated gives a default of UTC if the file doesn't exist + * Test that we get a default timezone of UTC if timedate1 isn't available on the bus */ -TEST_F(TimezoneFixture, DefaultValueNoFile) +TEST_F(Timedate1Fixture, DefaultTimezone) { - const std::string expected_timezone = "Etc/Utc"; - remove(TIMEZONE_FILE); - ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS)); + const std::string expected_tzid{"Etc/Utc"}; - expectLogMessage(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, "*No such file or directory*"); - TimedatedTimezone tz(TIMEZONE_FILE); - ASSERT_EQ(expected_timezone, tz.timezone.get()); + TimedatedTimezone tmp; + EXPECT_TRUE(wait_for_tzid(tmp, expected_tzid)) << "expected " << expected_tzid << " got " << tmp.timezone.get(); } /** - * Test that timezone-timedated picks up the initial value + * Test that we get the right tzid if timedated shows up on the bus BEFORE we start */ -TEST_F(TimezoneFixture, InitialValue) +TEST_F(Timedate1Fixture, Timedate1First) { - const std::string expected_timezone = "America/Chicago"; - set_file(expected_timezone); - TimedatedTimezone tz(TIMEZONE_FILE); + const std::string expected_tzid{"America/Chicago"}; + + start_timedate1(expected_tzid); + TimedatedTimezone tmp; + EXPECT_TRUE(wait_for_tzid(tmp, expected_tzid)) << "expected " << expected_tzid << " got " << tmp.timezone.get(); } /** - * Test that changing the tz after we are running works. + * Test that we get the right tzid if timedated shows up on the bus AFTER we start */ -TEST_F(TimezoneFixture, ChangedValue) +TEST_F(Timedate1Fixture, Timedate1Last) { - const std::string initial_timezone = "America/Chicago"; - const std::string changed_timezone = "America/New_York"; - - set_file(initial_timezone); + const std::string expected_tzid("America/Los_Angeles"); - TimedatedTimezone tz(TIMEZONE_FILE); - ASSERT_EQ(initial_timezone, tz.timezone.get()); - - bool changed = false; - tz.timezone.changed().connect( - [&changed, this](const std::string& s){ - g_message("timezone changed to %s", s.c_str()); - changed = true; - g_main_loop_quit(loop); - }); - - g_idle_add([](gpointer gself){ - static_cast(gself)->set_timezone("America/New_York"); - return G_SOURCE_REMOVE; - }, this); - - g_main_loop_run(loop); - - ASSERT_TRUE(changed); - ASSERT_EQ(changed_timezone, tz.timezone.get()); + TimedatedTimezone tmp; + start_timedate1(expected_tzid); + EXPECT_TRUE(wait_for_tzid(tmp, expected_tzid)) << "expected " << expected_tzid << " got " << tmp.timezone.get(); } /** - * Test that timezone-timedated picks up the initial value + * Test that the timezone core::Property changes when timedate1's property changes */ -TEST_F(TimezoneFixture, IgnoreComments) +TEST_F(Timedate1Fixture, TimezoneChange) { - const std::string comment = "# Created by cloud-init v. 0.7.5 on Thu, 24 Apr 2014 14:03:29 +0000"; - const std::string expected_timezone = "Europe/Berlin"; - set_file(comment + "\n" + expected_timezone); - TimedatedTimezone tz(TIMEZONE_FILE); - ASSERT_EQ(expected_timezone, tz.timezone.get()); + const std::vector expected_tzids{"America/Los_Angeles", "America/Chicago", "Etc/Utc"}; + + TimedatedTimezone tmp; + start_timedate1("America/New_York"); + + for(const auto& expected_tzid : expected_tzids) + { + set_timedate1_timezone(expected_tzid); + EXPECT_TRUE(wait_for_tzid(tmp, expected_tzid)) << "expected " << expected_tzid << " got " << tmp.timezone.get(); + } } -- cgit v1.2.3