From 132ac93bbd4141824cfa22f6516366f953fce9a0 Mon Sep 17 00:00:00 2001 From: Iain Lane Date: Mon, 31 Aug 2015 22:08:36 +0100 Subject: Use timedated's Timezone property instead of watching /etc/timezone Still need to rename everything to not use "timezone-file" --- include/datetime/timezone-file.h | 2 +- include/datetime/timezones-live.h | 2 +- src/CMakeLists.txt | 3 +- src/main.cpp | 4 +- src/timezone-file.cpp | 199 +++++++++++++------------ src/timezones-live.cpp | 5 +- tests/test-live-actions.cpp | 234 ++--------------------------- tests/test-timezone-file.cpp | 105 ++----------- tests/timedated-fixture.h | 302 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 445 insertions(+), 411 deletions(-) create mode 100644 tests/timedated-fixture.h diff --git a/include/datetime/timezone-file.h b/include/datetime/timezone-file.h index eca9c29..05dc7b0 100644 --- a/include/datetime/timezone-file.h +++ b/include/datetime/timezone-file.h @@ -34,7 +34,7 @@ namespace datetime { class FileTimezone: public Timezone { public: - FileTimezone(const std::string& filename); + FileTimezone(); ~FileTimezone(); private: diff --git a/include/datetime/timezones-live.h b/include/datetime/timezones-live.h index ca4ef31..49d9120 100644 --- a/include/datetime/timezones-live.h +++ b/include/datetime/timezones-live.h @@ -38,7 +38,7 @@ namespace datetime { class LiveTimezones: public Timezones { public: - LiveTimezones(const std::shared_ptr& settings, const std::string& filename); + LiveTimezones(const std::shared_ptr& settings); private: void update_geolocation(); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5fc822c..6f27e99 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,8 +1,7 @@ set (SERVICE_LIB "indicatordatetimeservice") set (SERVICE_EXEC "indicator-datetime-service") -add_definitions (-DTIMEZONE_FILE="/etc/timezone" - -DG_LOG_DOMAIN="Indicator-Datetime") +add_definitions (-DG_LOG_DOMAIN="Indicator-Datetime") # handwritten sources set (SERVICE_C_SOURCES diff --git a/src/main.cpp b/src/main.cpp index 907d49f..460f98c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -68,7 +68,7 @@ namespace { // create the live objects auto live_settings = std::make_shared(); - auto live_timezones = std::make_shared(live_settings, TIMEZONE_FILE); + auto live_timezones = std::make_shared(live_settings); auto live_clock = std::make_shared(timezone_); // create a full-month planner currently pointing to the current month @@ -128,7 +128,7 @@ main(int /*argc*/, char** /*argv*/) textdomain(GETTEXT_PACKAGE); auto engine = create_engine(); - auto timezone_ = std::make_shared(TIMEZONE_FILE); + auto timezone_ = std::make_shared(); auto state = create_state(engine, timezone_); auto actions = std::make_shared(state); MenuFactory factory(actions, state); diff --git a/src/timezone-file.cpp b/src/timezone-file.cpp index 3c73913..15e4f61 100644 --- a/src/timezone-file.cpp +++ b/src/timezone-file.cpp @@ -36,10 +36,11 @@ class FileTimezone::Impl { public: - Impl(FileTimezone& owner, const std::string& filename): - m_owner(owner) + Impl(FileTimezone& owner): + m_owner(owner), + m_loop(g_main_loop_new(nullptr, FALSE)) { - set_filename(filename); + monitor_timezone_property(); } ~Impl() @@ -51,113 +52,128 @@ private: void clear() { - if (m_monitor_handler_id) - g_signal_handler_disconnect(m_monitor, m_monitor_handler_id); - - g_clear_object (&m_monitor); - - m_filename.clear(); - } - - void set_filename(const std::string& filename) - { - clear(); - - auto tmp = realpath(filename.c_str(), nullptr); - if(tmp != nullptr) + if (m_bus_watch_id) { - m_filename = tmp; - free(tmp); - } - else - { - g_warning("Unable to resolve path '%s': %s", filename.c_str(), g_strerror(errno)); - m_filename = filename; // better than nothing? + g_bus_unwatch_name (m_bus_watch_id); + m_bus_watch_id = 0; } - auto file = g_file_new_for_path(m_filename.c_str()); - GError * err = nullptr; - m_monitor = g_file_monitor_file(file, G_FILE_MONITOR_NONE, nullptr, &err); - g_object_unref(file); - if (err) - { - g_warning("%s Unable to monitor timezone file '%s': %s", G_STRLOC, TIMEZONE_FILE, err->message); - g_error_free(err); - } - else + if (m_properties_changed_id) { - m_monitor_handler_id = g_signal_connect_swapped(m_monitor, "changed", G_CALLBACK(on_file_changed), this); - g_debug("%s Monitoring timezone file '%s'", G_STRLOC, m_filename.c_str()); + g_signal_handler_disconnect(m_proxy, m_properties_changed_id); + m_properties_changed_id = 0; } - reload(); + g_clear_object(&m_proxy); + g_clear_pointer(&m_loop, g_main_loop_unref); } - static void on_file_changed(gpointer gself) + static void on_properties_changed(GDBusProxy *proxy G_GNUC_UNUSED, + GVariant *changed_properties /* a{sv} */, + GStrv invalidated_properties G_GNUC_UNUSED, + gpointer gself) { - static_cast(gself)->reload(); + auto self = static_cast(gself); + char *tz; + + if (g_variant_lookup(changed_properties, "Timezone", "s", &tz, NULL)) + { + g_debug("on_properties_changed: got timezone '%s'", tz); + self->notify_timezone(tz); + g_free (tz); + } } - void reload() + static void on_proxy_ready(GObject *object G_GNUC_UNUSED, + GAsyncResult *res, + gpointer gself) { - const auto new_timezone = get_timezone_from_file(m_filename); + auto self = static_cast(gself); + GError *error = nullptr; + self->m_proxy = g_dbus_proxy_new_finish(res, &error); - if (!new_timezone.empty()) - m_owner.timezone.set(new_timezone); - } + if (error) + { + g_warning ("Couldn't create proxy to read timezone: %s", error->message); + goto out; + } - /*** - **** - ***/ + /* Read the property */ + GVariant *prop; + prop = g_dbus_proxy_get_cached_property(self->m_proxy, "Timezone"); - std::string get_timezone_from_file(const std::string& filename) - { - 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) + if (!prop || !g_variant_is_of_type(prop, G_VARIANT_TYPE_STRING)) { - 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_warning("Couldn't read the Timezone property, defaulting to Etc/Utc"); + self->notify_timezone("Etc/Utc"); + goto out; + } - g_strstrip(line->str); + const gchar *tz; + tz = g_variant_get_string(prop, nullptr); - if (!line->len) // skip empty lines - continue; + self->notify_timezone(tz); - if (*line->str=='#') // skip comments - continue; + self->m_properties_changed_id = g_signal_connect(self->m_proxy, + "g-properties-changed", + (GCallback) on_properties_changed, + gself); - ret = line->str; - } +out: + g_clear_pointer(&error, g_error_free); + g_clear_pointer(&prop, g_variant_unref); + if (self->m_loop && g_main_loop_is_running(self->m_loop)) + g_main_loop_quit(self->m_loop); + } - g_string_free(line, true); - } + static void on_name_appeared(GDBusConnection *connection, + const gchar *name, + const gchar *name_owner G_GNUC_UNUSED, + gpointer gself G_GNUC_UNUSED) + { + g_debug ("timedate1 appeared"); + g_dbus_proxy_new(connection, + G_DBUS_PROXY_FLAGS_NONE, + NULL, + name, + "/org/freedesktop/timedate1", + "org.freedesktop.timedate1", + nullptr, + on_proxy_ready, + gself); + } - if (io_channel != nullptr) - { - g_io_channel_shutdown(io_channel, false, nullptr); - g_io_channel_unref(io_channel); - } + static void on_name_vanished(GDBusConnection *connection G_GNUC_UNUSED, + const gchar *name G_GNUC_UNUSED, + gpointer gself) + { + auto self = static_cast(gself); + g_debug ("timedate1 vanished"); + + g_signal_handler_disconnect(self->m_proxy, + self->m_properties_changed_id); + self->m_properties_changed_id = 0; + g_clear_object(&self->m_proxy); + g_clear_pointer(&self->m_proxy, g_main_loop_unref); + } - if (error != nullptr) - { - g_warning("%s Unable to read timezone file '%s': %s", G_STRLOC, filename.c_str(), error->message); - g_error_free(error); - } + void monitor_timezone_property() + { + m_bus_watch_id = g_bus_watch_name (G_BUS_TYPE_SYSTEM, + "org.freedesktop.timedate1", + G_BUS_NAME_WATCHER_FLAGS_AUTO_START, + on_name_appeared, + on_name_vanished, + this, + nullptr); + + g_main_loop_run(m_loop); + } - return ret; + void notify_timezone(std::string new_timezone) + { + if (!new_timezone.empty()) + m_owner.timezone.set(new_timezone); } /*** @@ -165,17 +181,18 @@ private: ***/ FileTimezone & m_owner; - std::string m_filename; - GFileMonitor * m_monitor = nullptr; - unsigned long m_monitor_handler_id = 0; + unsigned long m_properties_changed_id = 0; + unsigned long m_bus_watch_id = 0; + GDBusProxy *m_proxy = nullptr; + GMainLoop *m_loop = nullptr; }; /*** **** ***/ -FileTimezone::FileTimezone(const std::string& filename): - impl(new Impl{*this, filename}) +FileTimezone::FileTimezone(): + impl(new Impl{*this}) { } diff --git a/src/timezones-live.cpp b/src/timezones-live.cpp index 4902b76..c5dd43c 100644 --- a/src/timezones-live.cpp +++ b/src/timezones-live.cpp @@ -25,9 +25,8 @@ namespace unity { namespace indicator { namespace datetime { -LiveTimezones::LiveTimezones(const std::shared_ptr& settings, - const std::string& filename): - m_file(filename), +LiveTimezones::LiveTimezones(const std::shared_ptr& settings): + m_file(), m_settings(settings) { m_file.timezone.changed().connect([this](const std::string&){update_timezones();}); diff --git a/tests/test-live-actions.cpp b/tests/test-live-actions.cpp index 4f84f25..9c74813 100644 --- a/tests/test-live-actions.cpp +++ b/tests/test-live-actions.cpp @@ -17,228 +17,18 @@ * Charles Kerr */ -#include - -#include "state-mock.h" -#include "glib-fixture.h" - -/*** -**** -***/ - -using namespace unity::indicator::datetime; - -class MockLiveActions: public LiveActions -{ -public: - std::string last_cmd; - std::string last_url; - explicit MockLiveActions(const std::shared_ptr& state_in): LiveActions(state_in) {} - ~MockLiveActions() {} - -protected: - void dispatch_url(const std::string& url) override { last_url = url; } - void execute_command(const std::string& cmd) override { last_cmd = cmd; } -}; - -/*** -**** -***/ - -using namespace unity::indicator::datetime; - -class LiveActionsFixture: public GlibFixture -{ -private: - - typedef GlibFixture super; - - static void on_bus_acquired(GDBusConnection* conn, - const gchar* name, - gpointer gself) - { - auto self = static_cast(gself); - g_debug("bus acquired: %s, connection is %p", name, conn); - - // Set up a mock GSD. - // All it really does is wait for calls to GetDevice and - // returns the get_devices_retval variant - static const GDBusInterfaceVTable vtable = { - timedate1_handle_method_call, - nullptr, /* GetProperty */ - nullptr, /* SetProperty */ - }; - - self->connection = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(conn))); - - GError* error = nullptr; - self->object_register_id = g_dbus_connection_register_object( - conn, - "/org/freedesktop/timedate1", - self->node_info->interfaces[0], - &vtable, - self, - nullptr, - &error); - g_assert_no_error(error); - } - - static void on_name_acquired(GDBusConnection* /*conn*/, - const gchar* /*name*/, - gpointer gself) - { - auto self = static_cast(gself); - self->name_acquired = true; - g_main_loop_quit(self->loop); - } - - static void on_name_lost(GDBusConnection* /*conn*/, - const gchar* /*name*/, - gpointer gself) - { - auto self = static_cast(gself); - self->name_acquired = false; - } - - static void on_bus_closed(GObject* /*object*/, - GAsyncResult* res, - gpointer gself) - { - auto self = static_cast(gself); - GError* err = nullptr; - g_dbus_connection_close_finish(self->connection, res, &err); - g_assert_no_error(err); - g_main_loop_quit(self->loop); - } - - static void - timedate1_handle_method_call(GDBusConnection * /*connection*/, - const gchar * /*sender*/, - const gchar * /*object_path*/, - const gchar * /*interface_name*/, - const gchar * method_name, - GVariant * parameters, - GDBusMethodInvocation * invocation, - gpointer gself) - { - g_assert(!g_strcmp0(method_name, "SetTimezone")); - g_assert(g_variant_is_of_type(parameters, G_VARIANT_TYPE_TUPLE)); - g_assert(2 == g_variant_n_children(parameters)); - - auto child = g_variant_get_child_value(parameters, 0); - g_assert(g_variant_is_of_type(child, G_VARIANT_TYPE_STRING)); - auto self = static_cast(gself); - self->attempted_tzid = g_variant_get_string(child, nullptr); - g_variant_unref(child); - - g_dbus_method_invocation_return_value(invocation, nullptr); - g_main_loop_quit(self->loop); - } - -protected: - - std::shared_ptr m_mock_state; - std::shared_ptr m_state; - std::shared_ptr m_live_actions; - std::shared_ptr m_actions; - - bool name_acquired; - std::string attempted_tzid; - - GTestDBus* bus; - guint own_name; - GDBusConnection* connection; - GDBusNodeInfo* node_info; - int object_register_id; - - void SetUp() - { - super::SetUp(); - - name_acquired = false; - attempted_tzid.clear(); - connection = nullptr; - node_info = nullptr; - object_register_id = 0; - own_name = 0; - - // bring up the test bus - bus = g_test_dbus_new(G_TEST_DBUS_NONE); - g_test_dbus_up(bus); - const auto address = g_test_dbus_get_bus_address(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); - - // parse the org.freedesktop.timedate1 interface - const gchar introspection_xml[] = - "" - " " - " " - " " - " " - " " - " " - ""; - node_info = g_dbus_node_info_new_for_xml(introspection_xml, nullptr); - ASSERT_TRUE(node_info != nullptr); - ASSERT_TRUE(node_info->interfaces != nullptr); - ASSERT_TRUE(node_info->interfaces[0] != nullptr); - ASSERT_TRUE(node_info->interfaces[1] == nullptr); - ASSERT_STREQ("org.freedesktop.timedate1", node_info->interfaces[0]->name); - - // own the bus - own_name = g_bus_own_name(G_BUS_TYPE_SYSTEM, - "org.freedesktop.timedate1", - G_BUS_NAME_OWNER_FLAGS_NONE, - on_bus_acquired, on_name_acquired, on_name_lost, - this, nullptr); - ASSERT_TRUE(object_register_id == 0); - ASSERT_FALSE(name_acquired); - ASSERT_TRUE(connection == nullptr); - g_main_loop_run(loop); - ASSERT_TRUE(object_register_id != 0); - ASSERT_TRUE(name_acquired); - ASSERT_TRUE(G_IS_DBUS_CONNECTION(connection)); - - // create the State and Actions - m_mock_state.reset(new MockState); - m_mock_state->settings.reset(new Settings); - m_state = std::dynamic_pointer_cast(m_mock_state); - m_live_actions.reset(new MockLiveActions(m_state)); - m_actions = std::dynamic_pointer_cast(m_live_actions); - } - - void TearDown() - { - m_actions.reset(); - m_live_actions.reset(); - m_state.reset(); - m_mock_state.reset(); - - g_dbus_connection_unregister_object(connection, object_register_id); - g_dbus_node_info_unref(node_info); - g_bus_unown_name(own_name); - g_dbus_connection_close(connection, nullptr, on_bus_closed, this); - g_main_loop_run(loop); - g_clear_object(&connection); - g_test_dbus_down(bus); - g_clear_object(&bus); - - super::TearDown(); - } -}; +#include "timedated-fixture.h" /*** **** ***/ -TEST_F(LiveActionsFixture, HelloWorld) +TEST_F(TimedateFixture, HelloWorld) { EXPECT_TRUE(true); } -TEST_F(LiveActionsFixture, SetLocation) +TEST_F(TimedateFixture, SetLocation) { const std::string tzid = "America/Chicago"; const std::string name = "Oklahoma City"; @@ -258,14 +48,14 @@ TEST_F(LiveActionsFixture, SetLocation) **** ***/ -TEST_F(LiveActionsFixture, DesktopOpenAlarmApp) +TEST_F(TimedateFixture, DesktopOpenAlarmApp) { m_actions->desktop_open_alarm_app(); const std::string expected = "evolution -c calendar"; EXPECT_EQ(expected, m_live_actions->last_cmd); } -TEST_F(LiveActionsFixture, DesktopOpenAppointment) +TEST_F(TimedateFixture, DesktopOpenAppointment) { Appointment a; a.uid = "some-uid"; @@ -275,14 +65,14 @@ TEST_F(LiveActionsFixture, DesktopOpenAppointment) EXPECT_NE(m_live_actions->last_cmd.find(expected_substr), std::string::npos); } -TEST_F(LiveActionsFixture, DesktopOpenCalendarApp) +TEST_F(TimedateFixture, DesktopOpenCalendarApp) { m_actions->desktop_open_calendar_app(DateTime::NowLocal()); const std::string expected_substr = "evolution \"calendar:///?startdate="; EXPECT_NE(m_live_actions->last_cmd.find(expected_substr), std::string::npos); } -TEST_F(LiveActionsFixture, DesktopOpenSettingsApp) +TEST_F(TimedateFixture, DesktopOpenSettingsApp) { m_actions->desktop_open_settings_app(); const std::string expected_substr = "control-center"; @@ -300,13 +90,13 @@ namespace const std::string calendar_app_url = "appid://com.ubuntu.calendar/calendar/current-user-version"; } -TEST_F(LiveActionsFixture, PhoneOpenAlarmApp) +TEST_F(TimedateFixture, PhoneOpenAlarmApp) { m_actions->phone_open_alarm_app(); EXPECT_EQ(clock_app_url, m_live_actions->last_url); } -TEST_F(LiveActionsFixture, PhoneOpenAppointment) +TEST_F(TimedateFixture, PhoneOpenAppointment) { Appointment a; @@ -321,14 +111,14 @@ TEST_F(LiveActionsFixture, PhoneOpenAppointment) EXPECT_EQ(clock_app_url, m_live_actions->last_url); } -TEST_F(LiveActionsFixture, PhoneOpenCalendarApp) +TEST_F(TimedateFixture, PhoneOpenCalendarApp) { m_actions->phone_open_calendar_app(DateTime::NowLocal()); const std::string expected = "appid://com.ubuntu.calendar/calendar/current-user-version"; EXPECT_EQ(expected, m_live_actions->last_url); } -TEST_F(LiveActionsFixture, PhoneOpenSettingsApp) +TEST_F(TimedateFixture, PhoneOpenSettingsApp) { m_actions->phone_open_settings_app(); const std::string expected = "settings:///system/time-date"; @@ -339,7 +129,7 @@ TEST_F(LiveActionsFixture, PhoneOpenSettingsApp) **** ***/ -TEST_F(LiveActionsFixture, CalendarState) +TEST_F(TimedateFixture, CalendarState) { // init the clock auto now = DateTime::Local(2014, 1, 1, 0, 0, 0); diff --git a/tests/test-timezone-file.cpp b/tests/test-timezone-file.cpp index 8968429..0ec496d 100644 --- a/tests/test-timezone-file.cpp +++ b/tests/test-timezone-file.cpp @@ -18,129 +18,56 @@ * with this program. If not, see . */ -#include "glib-fixture.h" +#include "timedated-fixture.h" #include -//#include -//#include -//#include -//#include -//#include -//#include -//#include -//#include - #include // fopen() //#include // chmod() #include // sync() using unity::indicator::datetime::FileTimezone; - -/*** -**** -***/ - -#define TIMEZONE_FILE (SANDBOX"/timezone") - -class TimezoneFixture: public GlibFixture -{ - private: - - typedef GlibFixture super; - - protected: - - void SetUp() override - { - super::SetUp(); - } - - void TearDown() override - { - super::TearDown(); - } - - public: - - /* convenience func to set the timezone file */ - void set_file(const std::string& text) - { - auto fp = fopen(TIMEZONE_FILE, "w+"); - fprintf(fp, "%s\n", text.c_str()); - fclose(fp); - sync(); - } -}; - - -/** - * Test that timezone-file 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)); - - FileTimezone tz(TIMEZONE_FILE); - testLogCount(G_LOG_LEVEL_WARNING, 1); -} - - /** * Test that timezone-file picks up the initial value */ -TEST_F(TimezoneFixture, InitialValue) +TEST_F(TimedateFixture, InitialValue) { const std::string expected_timezone = "America/Chicago"; - set_file(expected_timezone); - FileTimezone tz(TIMEZONE_FILE); + set_timezone(expected_timezone); + FileTimezone tz; ASSERT_EQ(expected_timezone, tz.timezone.get()); } - /** - * Test that clearing the timezone results in an empty string + * Test that changing the tz after we are running works. */ -TEST_F(TimezoneFixture, ChangedValue) +TEST_F(TimedateFixture, ChangedValue) { const std::string initial_timezone = "America/Chicago"; const std::string changed_timezone = "America/New_York"; - set_file(initial_timezone); + GMainLoop *l = g_main_loop_new(nullptr, FALSE); - FileTimezone tz(TIMEZONE_FILE); + set_timezone(initial_timezone); + + FileTimezone tz; ASSERT_EQ(initial_timezone, tz.timezone.get()); bool changed = false; - auto connection = tz.timezone.changed().connect( - [&changed, this](const std::string& s){ + tz.timezone.changed().connect( + [&changed, this, l](const std::string& s){ g_message("timezone changed to %s", s.c_str()); changed = true; - g_main_loop_quit(loop); + g_main_loop_quit(l); }); g_idle_add([](gpointer gself){ - static_cast(gself)->set_file("America/New_York"); - // static_cast(gtz)->timezone.set("America/New_York"); + static_cast(gself)->set_timezone("America/New_York"); return G_SOURCE_REMOVE; - }, this);//&tz); + }, this); - g_main_loop_run(loop); + g_main_loop_run(l); ASSERT_TRUE(changed); ASSERT_EQ(changed_timezone, tz.timezone.get()); } - - -/** - * Test that timezone-file picks up the initial value - */ -TEST_F(TimezoneFixture, IgnoreComments) -{ - 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); - FileTimezone tz(TIMEZONE_FILE); - ASSERT_EQ(expected_timezone, tz.timezone.get()); -} diff --git a/tests/timedated-fixture.h b/tests/timedated-fixture.h new file mode 100644 index 0000000..e75e6bd --- /dev/null +++ b/tests/timedated-fixture.h @@ -0,0 +1,302 @@ +/* + * Copyright 2013 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 + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, 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 . + * + * Authors: + * Charles Kerr + */ + +#ifndef INDICATOR_DATETIME_TESTS_TIMEDATED_FIXTURE_H +#define INDICATOR_DATETIME_TESTS_TIMEDATED_FIXTURE_H + +#include + +#include "state-mock.h" +#include "glib-fixture.h" + +using namespace unity::indicator::datetime; + +class MockLiveActions: public LiveActions +{ +public: + std::string last_cmd; + std::string last_url; + explicit MockLiveActions(const std::shared_ptr& state_in): LiveActions(state_in) {} + ~MockLiveActions() {} + +protected: + void dispatch_url(const std::string& url) override { last_url = url; } + void execute_command(const std::string& cmd) override { last_cmd = cmd; } +}; + +/*** +**** +***/ + +using namespace unity::indicator::datetime; + +class TimedateFixture: public GlibFixture +{ +private: + + typedef GlibFixture super; + + static GVariant * timedate1_get_properties (GDBusConnection * /*connection*/ , + const gchar * /*sender*/, + const gchar * /*object_path*/, + const gchar * /*interface_name*/, + const gchar *property_name, + GError ** /*error*/, + gpointer gself) + + { + auto self = static_cast(gself); + g_debug("get_properties called"); + if (g_strcmp0(property_name, "Timezone") == 0) + { + g_debug("timezone requested, giving '%s'", + self->attempted_tzid.c_str()); + return g_variant_new_string(self->attempted_tzid.c_str()); + } + return nullptr; + } + + + static void on_bus_acquired(GDBusConnection* conn, + const gchar* name, + gpointer gself) + { + auto self = static_cast(gself); + g_debug("bus acquired: %s, connection is %p", name, conn); + + // Set up a mock GSD. + // All it really does is wait for calls to GetDevice and + // returns the get_devices_retval variant + static const GDBusInterfaceVTable vtable = { + timedate1_handle_method_call, + timedate1_get_properties, /* GetProperty */ + nullptr, /* SetProperty */ + }; + + self->connection = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(conn))); + + GError* error = nullptr; + self->object_register_id = g_dbus_connection_register_object( + conn, + "/org/freedesktop/timedate1", + self->node_info->interfaces[0], + &vtable, + self, + nullptr, + &error); + g_assert_no_error(error); + } + + static void on_name_acquired(GDBusConnection* conn, + const gchar* name, + gpointer gself) + { + g_debug("on_name_acquired"); + auto self = static_cast(gself); + self->name_acquired = true; + self->proxy = g_dbus_proxy_new_sync(conn, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, + nullptr, + name, + "/org/freedesktop/timedate1", + "org.freedesktop.timedate1", + nullptr, + nullptr); + g_main_loop_quit(self->loop); + } + + static void on_name_lost(GDBusConnection* /*conn*/, + const gchar* /*name*/, + gpointer gself) + { + g_debug("on_name_lost"); + auto self = static_cast(gself); + self->name_acquired = false; + } + + static void on_bus_closed(GObject* /*object*/, + GAsyncResult* res, + gpointer gself) + { + g_debug("on_bus_closed"); + auto self = static_cast(gself); + GError* err = nullptr; + g_dbus_connection_close_finish(self->connection, res, &err); + g_assert_no_error(err); + g_main_loop_quit(self->loop); + } + + static void + timedate1_handle_method_call(GDBusConnection * connection, + const gchar * /*sender*/, + const gchar * object_path, + const gchar * interface_name, + const gchar * method_name, + GVariant * parameters, + GDBusMethodInvocation * invocation, + gpointer gself) + { + g_assert(!g_strcmp0(method_name, "SetTimezone")); + g_assert(g_variant_is_of_type(parameters, G_VARIANT_TYPE_TUPLE)); + g_assert(2 == g_variant_n_children(parameters)); + + auto child = g_variant_get_child_value(parameters, 0); + g_assert(g_variant_is_of_type(child, G_VARIANT_TYPE_STRING)); + auto self = static_cast(gself); + self->attempted_tzid = g_variant_get_string(child, nullptr); + g_debug("set tz (dbus side): '%s'", self->attempted_tzid.c_str()); + g_dbus_method_invocation_return_value(invocation, nullptr); + + /* Send PropertiesChanged */ + GError * local_error = nullptr; + auto builder = g_variant_builder_new (G_VARIANT_TYPE_ARRAY); + g_variant_builder_add (builder, + "{sv}", + "Timezone", + g_variant_new_string( + self->attempted_tzid.c_str())); + g_dbus_connection_emit_signal (connection, + NULL, + object_path, + "org.freedesktop.DBus.Properties", + "PropertiesChanged", + g_variant_new ("(sa{sv}as)", + interface_name, + builder, + NULL), + &local_error); + g_assert_no_error (local_error); + g_variant_unref(child); + + g_main_loop_quit(self->loop); + } + +protected: + + std::shared_ptr m_mock_state; + std::shared_ptr m_state; + std::shared_ptr m_live_actions; + std::shared_ptr m_actions; + + bool name_acquired; + std::string attempted_tzid; + + GTestDBus* bus; + guint own_name; + GDBusConnection* connection; + GDBusNodeInfo* node_info; + int object_register_id; + GDBusProxy *proxy; + + void SetUp() + { + super::SetUp(); + g_debug("SetUp"); + + name_acquired = false; + attempted_tzid.clear(); + connection = nullptr; + node_info = nullptr; + object_register_id = 0; + own_name = 0; + + // bring up the test bus + bus = g_test_dbus_new(G_TEST_DBUS_NONE); + g_test_dbus_up(bus); + const auto address = g_test_dbus_get_bus_address(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); + + // parse the org.freedesktop.timedate1 interface + const gchar introspection_xml[] = + "" + " " + " " + " " + " " + " " + " " + " " + ""; + node_info = g_dbus_node_info_new_for_xml(introspection_xml, nullptr); + ASSERT_TRUE(node_info != nullptr); + ASSERT_TRUE(node_info->interfaces != nullptr); + ASSERT_TRUE(node_info->interfaces[0] != nullptr); + ASSERT_TRUE(node_info->interfaces[1] == nullptr); + ASSERT_STREQ("org.freedesktop.timedate1", node_info->interfaces[0]->name); + + // own the bus + own_name = g_bus_own_name(G_BUS_TYPE_SYSTEM, + "org.freedesktop.timedate1", + G_BUS_NAME_OWNER_FLAGS_NONE, + on_bus_acquired, on_name_acquired, on_name_lost, + this, nullptr); + ASSERT_TRUE(object_register_id == 0); + ASSERT_FALSE(name_acquired); + ASSERT_TRUE(connection == nullptr); + g_main_loop_run(loop); + ASSERT_TRUE(object_register_id != 0); + ASSERT_TRUE(name_acquired); + ASSERT_TRUE(G_IS_DBUS_CONNECTION(connection)); + + // create the State and Actions + m_mock_state.reset(new MockState); + m_mock_state->settings.reset(new Settings); + m_state = std::dynamic_pointer_cast(m_mock_state); + m_live_actions.reset(new MockLiveActions(m_state)); + m_actions = std::dynamic_pointer_cast(m_live_actions); + } + + void TearDown() + { + g_debug("TearDown"); + m_actions.reset(); + m_live_actions.reset(); + m_state.reset(); + m_mock_state.reset(); + g_dbus_connection_unregister_object(connection, object_register_id); + g_object_unref(proxy); + g_dbus_node_info_unref(node_info); + g_bus_unown_name(own_name); + g_dbus_connection_close(connection, nullptr, on_bus_closed, this); + g_main_loop_run(loop); + g_clear_object(&connection); + g_test_dbus_down(bus); + g_clear_object(&bus); + + super::TearDown(); + } +public: + void set_timezone(std::string tz) + { + g_debug("set_timezone: '%s'", tz.c_str()); + g_dbus_proxy_call_sync(proxy, + "SetTimezone", + g_variant_new("(sb)", + tz.c_str(), + FALSE), + G_DBUS_CALL_FLAGS_NONE, + 500, + nullptr, + nullptr); + } +}; + +#endif -- cgit v1.2.3 From f016debfbaac39f509f5947b098c141a03a3b65f Mon Sep 17 00:00:00 2001 From: Iain Lane Date: Tue, 1 Sep 2015 10:25:41 +0100 Subject: Add a timeout so we can't hang forever --- src/timezone-file.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/timezone-file.cpp b/src/timezone-file.cpp index 15e4f61..ef9d78a 100644 --- a/src/timezone-file.cpp +++ b/src/timezone-file.cpp @@ -64,6 +64,12 @@ private: m_properties_changed_id = 0; } + if (m_timeout_id) + { + g_source_remove(m_timeout_id); + m_timeout_id = 0; + } + g_clear_object(&m_proxy); g_clear_pointer(&m_loop, g_main_loop_unref); } @@ -124,6 +130,12 @@ out: g_clear_pointer(&prop, g_variant_unref); if (self->m_loop && g_main_loop_is_running(self->m_loop)) g_main_loop_quit(self->m_loop); + + if (self->m_timeout_id) + { + g_source_remove(self->m_timeout_id); + self->m_timeout_id = 0; + } } static void on_name_appeared(GDBusConnection *connection, @@ -143,6 +155,20 @@ out: gself); } + static gboolean quit_loop(gpointer gself) + { + auto self = static_cast(gself); + + g_warning("Timed out when getting initial value of timezone, defaulting to UTC"); + self->notify_timezone("Etc/Utc"); + + g_main_loop_quit(self->m_loop); + + self->m_timeout_id = 0; + + return G_SOURCE_REMOVE; + } + static void on_name_vanished(GDBusConnection *connection G_GNUC_UNUSED, const gchar *name G_GNUC_UNUSED, gpointer gself) @@ -167,6 +193,8 @@ out: this, nullptr); + /* Incase something breaks, we don't want to hang */ + m_timeout_id = g_timeout_add(500, quit_loop, this); g_main_loop_run(m_loop); } @@ -183,6 +211,7 @@ out: FileTimezone & m_owner; unsigned long m_properties_changed_id = 0; unsigned long m_bus_watch_id = 0; + unsigned long m_timeout_id = 0; GDBusProxy *m_proxy = nullptr; GMainLoop *m_loop = nullptr; }; -- cgit v1.2.3 From a3937830bf656d5a8e2f368757b947cef0c8b1de Mon Sep 17 00:00:00 2001 From: Iain Lane Date: Tue, 1 Sep 2015 10:52:13 +0100 Subject: Rename FileTimezone to TimedatedTimezone --- include/datetime/timezone-file.h | 54 -------- include/datetime/timezone-timedated.h | 54 ++++++++ include/datetime/timezones-live.h | 6 +- src/CMakeLists.txt | 2 +- src/main.cpp | 4 +- src/timezone-file.cpp | 238 ---------------------------------- src/timezone-timedated.cpp | 238 ++++++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 2 +- tests/test-timezone-file.cpp | 73 ----------- tests/test-timezone-timedated.cpp | 73 +++++++++++ 10 files changed, 372 insertions(+), 372 deletions(-) delete mode 100644 include/datetime/timezone-file.h create mode 100644 include/datetime/timezone-timedated.h delete mode 100644 src/timezone-file.cpp create mode 100644 src/timezone-timedated.cpp delete mode 100644 tests/test-timezone-file.cpp create mode 100644 tests/test-timezone-timedated.cpp diff --git a/include/datetime/timezone-file.h b/include/datetime/timezone-file.h deleted file mode 100644 index 05dc7b0..0000000 --- a/include/datetime/timezone-file.h +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2013 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 - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranties of - * MERCHANTABILITY, SATISFACTORY QUALITY, 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 . - * - * Authors: - * Charles Kerr - */ - -#ifndef INDICATOR_DATETIME_FILE_TIMEZONE_H -#define INDICATOR_DATETIME_FILE_TIMEZONE_H - -#include // base class - -#include // std::string - -namespace unity { -namespace indicator { -namespace datetime { - -/** - * \brief A #Timezone that gets its information from monitoring a file, such as /etc/timezone - */ -class FileTimezone: public Timezone -{ -public: - FileTimezone(); - ~FileTimezone(); - -private: - class Impl; - friend Impl; - std::unique_ptr impl; - - // we have pointers in here, so disable copying - FileTimezone(const FileTimezone&) =delete; - FileTimezone& operator=(const FileTimezone&) =delete; -}; - -} // namespace datetime -} // namespace indicator -} // namespace unity - -#endif // INDICATOR_DATETIME_FILE_TIMEZONE_H diff --git a/include/datetime/timezone-timedated.h b/include/datetime/timezone-timedated.h new file mode 100644 index 0000000..3df9a3e --- /dev/null +++ b/include/datetime/timezone-timedated.h @@ -0,0 +1,54 @@ +/* + * Copyright 2013 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 + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, 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 . + * + * Authors: + * Charles Kerr + */ + +#ifndef INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H +#define INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H + +#include // base class + +#include // std::string + +namespace unity { +namespace indicator { +namespace datetime { + +/** + * \brief A #Timezone that gets its information from monitoring a file, such as /etc/timezone + */ +class TimedatedTimezone: public Timezone +{ +public: + TimedatedTimezone(); + ~TimedatedTimezone(); + +private: + class Impl; + friend Impl; + std::unique_ptr impl; + + // we have pointers in here, so disable copying + TimedatedTimezone(const TimedatedTimezone&) =delete; + TimedatedTimezone& operator=(const TimedatedTimezone&) =delete; +}; + +} // namespace datetime +} // namespace indicator +} // namespace unity + +#endif // INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H diff --git a/include/datetime/timezones-live.h b/include/datetime/timezones-live.h index 49d9120..b9d78a5 100644 --- a/include/datetime/timezones-live.h +++ b/include/datetime/timezones-live.h @@ -22,8 +22,8 @@ #include #include -#include #include +#include #include // shared_ptr<> @@ -32,7 +32,7 @@ namespace indicator { namespace datetime { /** - * \brief #Timezones object that uses a #FileTimezone and #GeoclueTimezone + * \brief #Timezones object that uses a #TimedatedTimezone and #GeoclueTimezone * to detect what timezone we're in */ class LiveTimezones: public Timezones @@ -44,7 +44,7 @@ private: void update_geolocation(); void update_timezones(); - FileTimezone m_file; + TimedatedTimezone m_file; std::shared_ptr m_settings; std::shared_ptr m_geo; }; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6f27e99..f8d219a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -33,9 +33,9 @@ set (SERVICE_CXX_SOURCES settings-live.cpp snap.cpp sound.cpp - timezone-file.cpp timezone-geoclue.cpp timezones-live.cpp + timezone-timedated.cpp utils.c wakeup-timer-mainloop.cpp wakeup-timer-powerd.cpp) diff --git a/src/main.cpp b/src/main.cpp index 460f98c..c7769c9 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -31,8 +31,8 @@ #include #include #include -#include #include +#include #include #include @@ -128,7 +128,7 @@ main(int /*argc*/, char** /*argv*/) textdomain(GETTEXT_PACKAGE); auto engine = create_engine(); - auto timezone_ = std::make_shared(); + auto timezone_ = std::make_shared(); auto state = create_state(engine, timezone_); auto actions = std::make_shared(state); MenuFactory factory(actions, state); diff --git a/src/timezone-file.cpp b/src/timezone-file.cpp deleted file mode 100644 index ef9d78a..0000000 --- a/src/timezone-file.cpp +++ /dev/null @@ -1,238 +0,0 @@ -/* - * Copyright 2013 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 - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranties of - * MERCHANTABILITY, SATISFACTORY QUALITY, 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 . - * - * Authors: - * Charles Kerr - */ - -#include - -#include - -#include -#include - -namespace unity { -namespace indicator { -namespace datetime { - -/*** -**** -***/ - -class FileTimezone::Impl -{ -public: - - Impl(FileTimezone& owner): - m_owner(owner), - m_loop(g_main_loop_new(nullptr, FALSE)) - { - monitor_timezone_property(); - } - - ~Impl() - { - clear(); - } - -private: - - void clear() - { - if (m_bus_watch_id) - { - g_bus_unwatch_name (m_bus_watch_id); - m_bus_watch_id = 0; - } - - if (m_properties_changed_id) - { - g_signal_handler_disconnect(m_proxy, m_properties_changed_id); - m_properties_changed_id = 0; - } - - if (m_timeout_id) - { - g_source_remove(m_timeout_id); - m_timeout_id = 0; - } - - g_clear_object(&m_proxy); - g_clear_pointer(&m_loop, g_main_loop_unref); - } - - static void on_properties_changed(GDBusProxy *proxy G_GNUC_UNUSED, - GVariant *changed_properties /* a{sv} */, - GStrv invalidated_properties G_GNUC_UNUSED, - gpointer gself) - { - auto self = static_cast(gself); - char *tz; - - if (g_variant_lookup(changed_properties, "Timezone", "s", &tz, NULL)) - { - g_debug("on_properties_changed: got timezone '%s'", tz); - self->notify_timezone(tz); - g_free (tz); - } - } - - static void on_proxy_ready(GObject *object G_GNUC_UNUSED, - GAsyncResult *res, - gpointer gself) - { - auto self = static_cast(gself); - GError *error = nullptr; - self->m_proxy = g_dbus_proxy_new_finish(res, &error); - - if (error) - { - g_warning ("Couldn't create proxy to read timezone: %s", error->message); - goto out; - } - - /* Read the property */ - GVariant *prop; - prop = g_dbus_proxy_get_cached_property(self->m_proxy, "Timezone"); - - if (!prop || !g_variant_is_of_type(prop, G_VARIANT_TYPE_STRING)) - { - g_warning("Couldn't read the Timezone property, defaulting to Etc/Utc"); - self->notify_timezone("Etc/Utc"); - goto out; - } - - const gchar *tz; - tz = g_variant_get_string(prop, nullptr); - - self->notify_timezone(tz); - - self->m_properties_changed_id = g_signal_connect(self->m_proxy, - "g-properties-changed", - (GCallback) on_properties_changed, - gself); - -out: - g_clear_pointer(&error, g_error_free); - g_clear_pointer(&prop, g_variant_unref); - if (self->m_loop && g_main_loop_is_running(self->m_loop)) - g_main_loop_quit(self->m_loop); - - if (self->m_timeout_id) - { - g_source_remove(self->m_timeout_id); - self->m_timeout_id = 0; - } - } - - static void on_name_appeared(GDBusConnection *connection, - const gchar *name, - const gchar *name_owner G_GNUC_UNUSED, - gpointer gself G_GNUC_UNUSED) - { - g_debug ("timedate1 appeared"); - g_dbus_proxy_new(connection, - G_DBUS_PROXY_FLAGS_NONE, - NULL, - name, - "/org/freedesktop/timedate1", - "org.freedesktop.timedate1", - nullptr, - on_proxy_ready, - gself); - } - - static gboolean quit_loop(gpointer gself) - { - auto self = static_cast(gself); - - g_warning("Timed out when getting initial value of timezone, defaulting to UTC"); - self->notify_timezone("Etc/Utc"); - - g_main_loop_quit(self->m_loop); - - self->m_timeout_id = 0; - - return G_SOURCE_REMOVE; - } - - static void on_name_vanished(GDBusConnection *connection G_GNUC_UNUSED, - const gchar *name G_GNUC_UNUSED, - gpointer gself) - { - auto self = static_cast(gself); - g_debug ("timedate1 vanished"); - - g_signal_handler_disconnect(self->m_proxy, - self->m_properties_changed_id); - self->m_properties_changed_id = 0; - g_clear_object(&self->m_proxy); - g_clear_pointer(&self->m_proxy, g_main_loop_unref); - } - - void monitor_timezone_property() - { - m_bus_watch_id = g_bus_watch_name (G_BUS_TYPE_SYSTEM, - "org.freedesktop.timedate1", - G_BUS_NAME_WATCHER_FLAGS_AUTO_START, - on_name_appeared, - on_name_vanished, - this, - nullptr); - - /* Incase something breaks, we don't want to hang */ - m_timeout_id = g_timeout_add(500, quit_loop, this); - g_main_loop_run(m_loop); - } - - void notify_timezone(std::string new_timezone) - { - if (!new_timezone.empty()) - m_owner.timezone.set(new_timezone); - } - - /*** - **** - ***/ - - FileTimezone & m_owner; - unsigned long m_properties_changed_id = 0; - unsigned long m_bus_watch_id = 0; - unsigned long m_timeout_id = 0; - GDBusProxy *m_proxy = nullptr; - GMainLoop *m_loop = nullptr; -}; - -/*** -**** -***/ - -FileTimezone::FileTimezone(): - impl(new Impl{*this}) -{ -} - -FileTimezone::~FileTimezone() -{ -} - -/*** -**** -***/ - -} // namespace datetime -} // namespace indicator -} // namespace unity diff --git a/src/timezone-timedated.cpp b/src/timezone-timedated.cpp new file mode 100644 index 0000000..778697a --- /dev/null +++ b/src/timezone-timedated.cpp @@ -0,0 +1,238 @@ +/* + * Copyright 2013 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 + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, 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 . + * + * Authors: + * Charles Kerr + */ + +#include + +#include + +#include +#include + +namespace unity { +namespace indicator { +namespace datetime { + +/*** +**** +***/ + +class TimedatedTimezone::Impl +{ +public: + + Impl(TimedatedTimezone& owner): + m_owner(owner), + m_loop(g_main_loop_new(nullptr, FALSE)) + { + monitor_timezone_property(); + } + + ~Impl() + { + clear(); + } + +private: + + void clear() + { + if (m_bus_watch_id) + { + g_bus_unwatch_name (m_bus_watch_id); + m_bus_watch_id = 0; + } + + if (m_properties_changed_id) + { + g_signal_handler_disconnect(m_proxy, m_properties_changed_id); + m_properties_changed_id = 0; + } + + if (m_timeout_id) + { + g_source_remove(m_timeout_id); + m_timeout_id = 0; + } + + g_clear_object(&m_proxy); + g_clear_pointer(&m_loop, g_main_loop_unref); + } + + static void on_properties_changed(GDBusProxy *proxy G_GNUC_UNUSED, + GVariant *changed_properties /* a{sv} */, + GStrv invalidated_properties G_GNUC_UNUSED, + gpointer gself) + { + auto self = static_cast(gself); + char *tz; + + if (g_variant_lookup(changed_properties, "Timezone", "s", &tz, NULL)) + { + g_debug("on_properties_changed: got timezone '%s'", tz); + self->notify_timezone(tz); + g_free (tz); + } + } + + static void on_proxy_ready(GObject *object G_GNUC_UNUSED, + GAsyncResult *res, + gpointer gself) + { + auto self = static_cast(gself); + GError *error = nullptr; + self->m_proxy = g_dbus_proxy_new_finish(res, &error); + + if (error) + { + g_warning ("Couldn't create proxy to read timezone: %s", error->message); + goto out; + } + + /* Read the property */ + GVariant *prop; + prop = g_dbus_proxy_get_cached_property(self->m_proxy, "Timezone"); + + if (!prop || !g_variant_is_of_type(prop, G_VARIANT_TYPE_STRING)) + { + g_warning("Couldn't read the Timezone property, defaulting to Etc/Utc"); + self->notify_timezone("Etc/Utc"); + goto out; + } + + const gchar *tz; + tz = g_variant_get_string(prop, nullptr); + + self->notify_timezone(tz); + + self->m_properties_changed_id = g_signal_connect(self->m_proxy, + "g-properties-changed", + (GCallback) on_properties_changed, + gself); + +out: + g_clear_pointer(&error, g_error_free); + g_clear_pointer(&prop, g_variant_unref); + if (self->m_loop && g_main_loop_is_running(self->m_loop)) + g_main_loop_quit(self->m_loop); + + if (self->m_timeout_id) + { + g_source_remove(self->m_timeout_id); + self->m_timeout_id = 0; + } + } + + static void on_name_appeared(GDBusConnection *connection, + const gchar *name, + const gchar *name_owner G_GNUC_UNUSED, + gpointer gself G_GNUC_UNUSED) + { + g_debug ("timedate1 appeared"); + g_dbus_proxy_new(connection, + G_DBUS_PROXY_FLAGS_NONE, + NULL, + name, + "/org/freedesktop/timedate1", + "org.freedesktop.timedate1", + nullptr, + on_proxy_ready, + gself); + } + + static gboolean quit_loop(gpointer gself) + { + auto self = static_cast(gself); + + g_warning("Timed out when getting initial value of timezone, defaulting to UTC"); + self->notify_timezone("Etc/Utc"); + + g_main_loop_quit(self->m_loop); + + self->m_timeout_id = 0; + + return G_SOURCE_REMOVE; + } + + static void on_name_vanished(GDBusConnection *connection G_GNUC_UNUSED, + const gchar *name G_GNUC_UNUSED, + gpointer gself) + { + auto self = static_cast(gself); + g_debug ("timedate1 vanished"); + + g_signal_handler_disconnect(self->m_proxy, + self->m_properties_changed_id); + self->m_properties_changed_id = 0; + g_clear_object(&self->m_proxy); + g_clear_pointer(&self->m_proxy, g_main_loop_unref); + } + + void monitor_timezone_property() + { + m_bus_watch_id = g_bus_watch_name (G_BUS_TYPE_SYSTEM, + "org.freedesktop.timedate1", + G_BUS_NAME_WATCHER_FLAGS_AUTO_START, + on_name_appeared, + on_name_vanished, + this, + nullptr); + + /* Incase something breaks, we don't want to hang */ + m_timeout_id = g_timeout_add(500, quit_loop, this); + g_main_loop_run(m_loop); + } + + void notify_timezone(std::string new_timezone) + { + if (!new_timezone.empty()) + m_owner.timezone.set(new_timezone); + } + + /*** + **** + ***/ + + TimedatedTimezone & m_owner; + unsigned long m_properties_changed_id = 0; + unsigned long m_bus_watch_id = 0; + unsigned long m_timeout_id = 0; + GDBusProxy *m_proxy = nullptr; + GMainLoop *m_loop = nullptr; +}; + +/*** +**** +***/ + +TimedatedTimezone::TimedatedTimezone(): + impl(new Impl{*this}) +{ +} + +TimedatedTimezone::~TimedatedTimezone() +{ +} + +/*** +**** +***/ + +} // namespace datetime +} // namespace indicator +} // namespace unity diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9d26484..032b84e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -58,7 +58,7 @@ add_test_by_name(test-locations) add_test_by_name(test-menus) add_test_by_name(test-planner) add_test_by_name(test-settings) -add_test_by_name(test-timezone-file) +add_test_by_name(test-timezone-timedated) add_test_by_name(test-utils) set (TEST_NAME manual-test-snap) diff --git a/tests/test-timezone-file.cpp b/tests/test-timezone-file.cpp deleted file mode 100644 index 0ec496d..0000000 --- a/tests/test-timezone-file.cpp +++ /dev/null @@ -1,73 +0,0 @@ - -/* - * Copyright 2013 Canonical Ltd. - * - * Authors: - * Charles Kerr - * - * 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 - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranties of - * MERCHANTABILITY, SATISFACTORY QUALITY, 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 . - */ - -#include "timedated-fixture.h" - -#include - -#include // fopen() -//#include // chmod() -#include // sync() - -using unity::indicator::datetime::FileTimezone; - -/** - * Test that timezone-file picks up the initial value - */ -TEST_F(TimedateFixture, InitialValue) -{ - const std::string expected_timezone = "America/Chicago"; - set_timezone(expected_timezone); - FileTimezone tz; - ASSERT_EQ(expected_timezone, tz.timezone.get()); -} - -/** - * Test that changing the tz after we are running works. - */ -TEST_F(TimedateFixture, ChangedValue) -{ - const std::string initial_timezone = "America/Chicago"; - const std::string changed_timezone = "America/New_York"; - GMainLoop *l = g_main_loop_new(nullptr, FALSE); - - set_timezone(initial_timezone); - - FileTimezone tz; - ASSERT_EQ(initial_timezone, tz.timezone.get()); - - bool changed = false; - tz.timezone.changed().connect( - [&changed, this, l](const std::string& s){ - g_message("timezone changed to %s", s.c_str()); - changed = true; - g_main_loop_quit(l); - }); - - g_idle_add([](gpointer gself){ - static_cast(gself)->set_timezone("America/New_York"); - return G_SOURCE_REMOVE; - }, this); - - g_main_loop_run(l); - - ASSERT_TRUE(changed); - ASSERT_EQ(changed_timezone, tz.timezone.get()); -} diff --git a/tests/test-timezone-timedated.cpp b/tests/test-timezone-timedated.cpp new file mode 100644 index 0000000..7086e96 --- /dev/null +++ b/tests/test-timezone-timedated.cpp @@ -0,0 +1,73 @@ + +/* + * Copyright 2013 Canonical Ltd. + * + * Authors: + * Charles Kerr + * + * 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 + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, 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 . + */ + +#include "timedated-fixture.h" + +#include + +#include // fopen() +//#include // chmod() +#include // sync() + +using unity::indicator::datetime::TimedatedTimezone; + +/** + * Test that timezone-file picks up the initial value + */ +TEST_F(TimedateFixture, InitialValue) +{ + const std::string expected_timezone = "America/Chicago"; + set_timezone(expected_timezone); + TimedatedTimezone tz; + ASSERT_EQ(expected_timezone, tz.timezone.get()); +} + +/** + * Test that changing the tz after we are running works. + */ +TEST_F(TimedateFixture, ChangedValue) +{ + const std::string initial_timezone = "America/Chicago"; + const std::string changed_timezone = "America/New_York"; + GMainLoop *l = g_main_loop_new(nullptr, FALSE); + + set_timezone(initial_timezone); + + TimedatedTimezone tz; + ASSERT_EQ(initial_timezone, tz.timezone.get()); + + bool changed = false; + tz.timezone.changed().connect( + [&changed, this, l](const std::string& s){ + g_message("timezone changed to %s", s.c_str()); + changed = true; + g_main_loop_quit(l); + }); + + g_idle_add([](gpointer gself){ + static_cast(gself)->set_timezone("America/New_York"); + return G_SOURCE_REMOVE; + }, this); + + g_main_loop_run(l); + + ASSERT_TRUE(changed); + ASSERT_EQ(changed_timezone, tz.timezone.get()); +} -- cgit v1.2.3 From 1da27f9088a759e78e88a390583d8646f2a82656 Mon Sep 17 00:00:00 2001 From: Iain Lane Date: Tue, 1 Sep 2015 11:10:09 +0100 Subject: Add some comments --- src/timezone-timedated.cpp | 2 ++ tests/timedated-fixture.h | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/timezone-timedated.cpp b/src/timezone-timedated.cpp index 778697a..bfe38f4 100644 --- a/src/timezone-timedated.cpp +++ b/src/timezone-timedated.cpp @@ -195,6 +195,8 @@ out: /* Incase something breaks, we don't want to hang */ m_timeout_id = g_timeout_add(500, quit_loop, this); + + /* We need to block until we've read the tz once */ g_main_loop_run(m_loop); } diff --git a/tests/timedated-fixture.h b/tests/timedated-fixture.h index e75e6bd..0e89ea1 100644 --- a/tests/timedated-fixture.h +++ b/tests/timedated-fixture.h @@ -80,9 +80,9 @@ private: auto self = static_cast(gself); g_debug("bus acquired: %s, connection is %p", name, conn); - // Set up a mock GSD. - // All it really does is wait for calls to GetDevice and - // returns the get_devices_retval variant + /* Set up a fake timedated which handles setting and getting the + ** timezone + */ static const GDBusInterfaceVTable vtable = { timedate1_handle_method_call, timedate1_get_properties, /* GetProperty */ -- cgit v1.2.3 From 29b5c4da1e44534352d29536bb6ad1c721406b8a Mon Sep 17 00:00:00 2001 From: Iain Lane Date: Thu, 3 Sep 2015 10:54:20 +0100 Subject: Avoid nested GMainLoops by reading from the file on startup Restore some tests for this functionality. --- include/datetime/timezone-timedated.h | 4 +- src/timezone-timedated.cpp | 128 +++++++++++++++++++++++----------- tests/test-timezone-timedated.cpp | 100 +++++++++++++++++++++----- tests/timedated-fixture.h | 3 +- 4 files changed, 174 insertions(+), 61 deletions(-) diff --git a/include/datetime/timezone-timedated.h b/include/datetime/timezone-timedated.h index 3df9a3e..5978e3e 100644 --- a/include/datetime/timezone-timedated.h +++ b/include/datetime/timezone-timedated.h @@ -20,6 +20,8 @@ #ifndef INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H #define INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H +#define DEFAULT_FILENAME "/etc/timezone" + #include // base class #include // std::string @@ -34,7 +36,7 @@ namespace datetime { class TimedatedTimezone: public Timezone { public: - TimedatedTimezone(); + TimedatedTimezone(std::string filename = DEFAULT_FILENAME); ~TimedatedTimezone(); private: diff --git a/src/timezone-timedated.cpp b/src/timezone-timedated.cpp index bfe38f4..a2918f0 100644 --- a/src/timezone-timedated.cpp +++ b/src/timezone-timedated.cpp @@ -36,10 +36,11 @@ class TimedatedTimezone::Impl { public: - Impl(TimedatedTimezone& owner): + Impl(TimedatedTimezone& owner, std::string filename): m_owner(owner), - m_loop(g_main_loop_new(nullptr, FALSE)) + m_filename(filename) { + g_debug("Filename is '%s'", filename.c_str()); monitor_timezone_property(); } @@ -64,14 +65,7 @@ private: m_properties_changed_id = 0; } - if (m_timeout_id) - { - g_source_remove(m_timeout_id); - m_timeout_id = 0; - } - g_clear_object(&m_proxy); - g_clear_pointer(&m_loop, g_main_loop_unref); } static void on_properties_changed(GDBusProxy *proxy G_GNUC_UNUSED, @@ -128,14 +122,6 @@ private: out: g_clear_pointer(&error, g_error_free); g_clear_pointer(&prop, g_variant_unref); - if (self->m_loop && g_main_loop_is_running(self->m_loop)) - g_main_loop_quit(self->m_loop); - - if (self->m_timeout_id) - { - g_source_remove(self->m_timeout_id); - self->m_timeout_id = 0; - } } static void on_name_appeared(GDBusConnection *connection, @@ -155,20 +141,6 @@ out: gself); } - static gboolean quit_loop(gpointer gself) - { - auto self = static_cast(gself); - - g_warning("Timed out when getting initial value of timezone, defaulting to UTC"); - self->notify_timezone("Etc/Utc"); - - g_main_loop_quit(self->m_loop); - - self->m_timeout_id = 0; - - return G_SOURCE_REMOVE; - } - static void on_name_vanished(GDBusConnection *connection G_GNUC_UNUSED, const gchar *name G_GNUC_UNUSED, gpointer gself) @@ -185,27 +157,100 @@ out: void monitor_timezone_property() { - m_bus_watch_id = g_bus_watch_name (G_BUS_TYPE_SYSTEM, + GError *err = nullptr; + GDBusConnection *conn; + + /* + * 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. + */ + conn = g_bus_get_sync(G_BUS_TYPE_SYSTEM, + nullptr, + &err); + + if (err) + { + g_warning("Couldn't get bus connection: '%s'", err->message); + g_error_free(err); + return; + } + + m_bus_watch_id = g_bus_watch_name_on_connection(conn, "org.freedesktop.timedate1", - G_BUS_NAME_WATCHER_FLAGS_AUTO_START, + G_BUS_NAME_WATCHER_FLAGS_NONE, on_name_appeared, on_name_vanished, this, nullptr); - /* Incase something breaks, we don't want to hang */ - m_timeout_id = g_timeout_add(500, quit_loop, this); - - /* We need to block until we've read the tz once */ - g_main_loop_run(m_loop); + g_object_unref (conn); } void notify_timezone(std::string new_timezone) { + g_debug("notify_timezone '%s'", new_timezone.c_str()); if (!new_timezone.empty()) m_owner.timezone.set(new_timezone); } + std::string get_timezone_from_file(const std::string& filename) + { + 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); + + if (!line->len) // skip empty lines + continue; + + if (*line->str=='#') // skip comments + continue; + + ret = line->str; + } + + g_string_free(line, true); + } else + /* Default to UTC */ + ret = "Etc/Utc"; + + if (io_channel != nullptr) + { + g_io_channel_shutdown(io_channel, false, nullptr); + g_io_channel_unref(io_channel); + } + + if (error != nullptr) + { + g_warning("%s Unable to read timezone file '%s': %s", G_STRLOC, filename.c_str(), error->message); + g_error_free(error); + } + + return ret; + } + /*** **** ***/ @@ -213,17 +258,16 @@ out: TimedatedTimezone & m_owner; unsigned long m_properties_changed_id = 0; unsigned long m_bus_watch_id = 0; - unsigned long m_timeout_id = 0; GDBusProxy *m_proxy = nullptr; - GMainLoop *m_loop = nullptr; + std::string m_filename; }; /*** **** ***/ -TimedatedTimezone::TimedatedTimezone(): - impl(new Impl{*this}) +TimedatedTimezone::TimedatedTimezone(std::string filename): + impl(new Impl{*this, filename}) { } diff --git a/tests/test-timezone-timedated.cpp b/tests/test-timezone-timedated.cpp index 7086e96..5cfc311 100644 --- a/tests/test-timezone-timedated.cpp +++ b/tests/test-timezone-timedated.cpp @@ -22,43 +22,99 @@ #include -#include // fopen() -//#include // chmod() -#include // sync() - using unity::indicator::datetime::TimedatedTimezone; +/*** +**** +***/ + +#define TIMEZONE_FILE (SANDBOX"/timezone") + +class TimezoneFixture: public TimedateFixture +{ + private: + + typedef TimedateFixture super; + + protected: + + void SetUp() override + { + super::SetUp(); + } + + void TearDown() override + { + super::TearDown(); + } + + public: + + /* convenience func to set the timezone file */ + void set_file(const std::string& text) + { + 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(); + } +}; + /** - * Test that timezone-file picks up the initial value + * Test that timezone-timedated warns, but doesn't crash, if the timezone file doesn't exist */ -TEST_F(TimedateFixture, InitialValue) +TEST_F(TimezoneFixture, NoFile) { - const std::string expected_timezone = "America/Chicago"; - set_timezone(expected_timezone); - TimedatedTimezone tz; + remove(TIMEZONE_FILE); + ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS)); + + TimedatedTimezone tz(TIMEZONE_FILE); + testLogCount(G_LOG_LEVEL_WARNING, 1); +} + +/** + * Test that timezone-timedated gives a default of UTC if the file doesn't exist + */ +TEST_F(TimezoneFixture, DefaultValueNoFile) +{ + const std::string expected_timezone = "Etc/Utc"; + remove(TIMEZONE_FILE); + ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS)); + + TimedatedTimezone tz(TIMEZONE_FILE); ASSERT_EQ(expected_timezone, tz.timezone.get()); } +/** + * Test that timezone-timedated picks up the initial value + */ +TEST_F(TimezoneFixture, InitialValue) +{ + const std::string expected_timezone = "America/Chicago"; + set_file(expected_timezone); + TimedatedTimezone tz(TIMEZONE_FILE); +} + /** * Test that changing the tz after we are running works. */ -TEST_F(TimedateFixture, ChangedValue) +TEST_F(TimezoneFixture, ChangedValue) { const std::string initial_timezone = "America/Chicago"; const std::string changed_timezone = "America/New_York"; - GMainLoop *l = g_main_loop_new(nullptr, FALSE); - set_timezone(initial_timezone); + set_file(initial_timezone); - TimedatedTimezone tz; + TimedatedTimezone tz(TIMEZONE_FILE); ASSERT_EQ(initial_timezone, tz.timezone.get()); bool changed = false; tz.timezone.changed().connect( - [&changed, this, l](const std::string& s){ + [&changed, this](const std::string& s){ g_message("timezone changed to %s", s.c_str()); changed = true; - g_main_loop_quit(l); + g_main_loop_quit(loop); }); g_idle_add([](gpointer gself){ @@ -66,8 +122,20 @@ TEST_F(TimedateFixture, ChangedValue) return G_SOURCE_REMOVE; }, this); - g_main_loop_run(l); + g_main_loop_run(loop); ASSERT_TRUE(changed); ASSERT_EQ(changed_timezone, tz.timezone.get()); } + +/** + * Test that timezone-timedated picks up the initial value + */ +TEST_F(TimezoneFixture, IgnoreComments) +{ + 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()); +} diff --git a/tests/timedated-fixture.h b/tests/timedated-fixture.h index 0e89ea1..5ec425d 100644 --- a/tests/timedated-fixture.h +++ b/tests/timedated-fixture.h @@ -183,8 +183,6 @@ private: &local_error); g_assert_no_error (local_error); g_variant_unref(child); - - g_main_loop_quit(self->loop); } protected: @@ -215,6 +213,7 @@ protected: node_info = nullptr; object_register_id = 0; own_name = 0; + proxy = nullptr; // bring up the test bus bus = g_test_dbus_new(G_TEST_DBUS_NONE); -- cgit v1.2.3 From 49dbcaf901fb3d5fba29f616d1e57a286e85bae8 Mon Sep 17 00:00:00 2001 From: Iain Lane Date: Thu, 3 Sep 2015 11:02:27 +0100 Subject: Make test-live-actions quit itself once the tz has been changed, the mock no longer does this --- tests/test-live-actions.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test-live-actions.cpp b/tests/test-live-actions.cpp index 9c74813..9f17001 100644 --- a/tests/test-live-actions.cpp +++ b/tests/test-live-actions.cpp @@ -37,6 +37,10 @@ TEST_F(TimedateFixture, SetLocation) EXPECT_NE(expected, m_state->settings->timezone_name.get()); m_actions->set_location(tzid, name); + m_state->settings->timezone_name.changed().connect( + [this](const std::string&){ + g_main_loop_quit(loop); + }); g_main_loop_run(loop); EXPECT_EQ(attempted_tzid, tzid); wait_msec(); -- cgit v1.2.3 From bda03d74c5a36bdaac89a64ff73fcc92dbd795a0 Mon Sep 17 00:00:00 2001 From: Lars Uebernickel Date: Thu, 3 Sep 2015 18:00:36 +0200 Subject: timezone-timedated: subscribe to PropertiesChanged directly This avoids some unnecessary calls that GDBusProxy makes and the name watching. --- src/timezone-timedated.cpp | 132 +++++++++++---------------------------------- 1 file changed, 30 insertions(+), 102 deletions(-) diff --git a/src/timezone-timedated.cpp b/src/timezone-timedated.cpp index a2918f0..fa99cd2 100644 --- a/src/timezone-timedated.cpp +++ b/src/timezone-timedated.cpp @@ -53,112 +53,42 @@ private: void clear() { - if (m_bus_watch_id) + if (m_connection && m_signal_subscription_id) { - g_bus_unwatch_name (m_bus_watch_id); - m_bus_watch_id = 0; + g_dbus_connection_signal_unsubscribe (m_connection, m_signal_subscription_id); + m_signal_subscription_id = 0; } - if (m_properties_changed_id) - { - g_signal_handler_disconnect(m_proxy, m_properties_changed_id); - m_properties_changed_id = 0; - } - - g_clear_object(&m_proxy); - } - - static void on_properties_changed(GDBusProxy *proxy G_GNUC_UNUSED, - GVariant *changed_properties /* a{sv} */, - GStrv invalidated_properties G_GNUC_UNUSED, - gpointer gself) - { - auto self = static_cast(gself); - char *tz; - - if (g_variant_lookup(changed_properties, "Timezone", "s", &tz, NULL)) - { - g_debug("on_properties_changed: got timezone '%s'", tz); - self->notify_timezone(tz); - g_free (tz); - } + g_clear_object(&m_connection); } - static void on_proxy_ready(GObject *object G_GNUC_UNUSED, - GAsyncResult *res, + 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); - GError *error = nullptr; - self->m_proxy = g_dbus_proxy_new_finish(res, &error); + const char *tz; + GVariant *changed_properties; + gchar **invalidated_properties; - if (error) - { - g_warning ("Couldn't create proxy to read timezone: %s", error->message); - goto out; - } - - /* Read the property */ - GVariant *prop; - prop = g_dbus_proxy_get_cached_property(self->m_proxy, "Timezone"); - - if (!prop || !g_variant_is_of_type(prop, G_VARIANT_TYPE_STRING)) - { - g_warning("Couldn't read the Timezone property, defaulting to Etc/Utc"); - self->notify_timezone("Etc/Utc"); - goto out; - } + g_variant_get (parameters, "(s@a{sv}^as)", NULL, &changed_properties, &invalidated_properties); - const gchar *tz; - tz = g_variant_get_string(prop, nullptr); - - self->notify_timezone(tz); - - self->m_properties_changed_id = g_signal_connect(self->m_proxy, - "g-properties-changed", - (GCallback) on_properties_changed, - gself); - -out: - g_clear_pointer(&error, g_error_free); - g_clear_pointer(&prop, g_variant_unref); - } - - static void on_name_appeared(GDBusConnection *connection, - const gchar *name, - const gchar *name_owner G_GNUC_UNUSED, - gpointer gself G_GNUC_UNUSED) - { - g_debug ("timedate1 appeared"); - g_dbus_proxy_new(connection, - G_DBUS_PROXY_FLAGS_NONE, - NULL, - name, - "/org/freedesktop/timedate1", - "org.freedesktop.timedate1", - nullptr, - on_proxy_ready, - gself); - } - - static void on_name_vanished(GDBusConnection *connection G_GNUC_UNUSED, - const gchar *name G_GNUC_UNUSED, - gpointer gself) - { - auto self = static_cast(gself); - g_debug ("timedate1 vanished"); + 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)); - g_signal_handler_disconnect(self->m_proxy, - self->m_properties_changed_id); - self->m_properties_changed_id = 0; - g_clear_object(&self->m_proxy); - g_clear_pointer(&self->m_proxy, g_main_loop_unref); + g_variant_unref (changed_properties); + g_strfreev (invalidated_properties); } void monitor_timezone_property() { GError *err = nullptr; - GDBusConnection *conn; /* * There is an unlikely race which happens if there is an activation @@ -170,7 +100,7 @@ out: * Make sure the bus is around at least until we add the match rules, * otherwise things (tests) are sad. */ - conn = g_bus_get_sync(G_BUS_TYPE_SYSTEM, + m_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, &err); @@ -181,15 +111,14 @@ out: return; } - m_bus_watch_id = g_bus_watch_name_on_connection(conn, + m_signal_subscription_id = g_dbus_connection_signal_subscribe(m_connection, "org.freedesktop.timedate1", - G_BUS_NAME_WATCHER_FLAGS_NONE, - on_name_appeared, - on_name_vanished, - this, - nullptr); - - g_object_unref (conn); + "org.freedesktop.DBus.Properties", + "PropertiesChanged", + "/org/freedesktop/timedate1", + NULL, G_DBUS_SIGNAL_FLAGS_NONE, + on_properties_changed, + this, nullptr); } void notify_timezone(std::string new_timezone) @@ -256,9 +185,8 @@ out: ***/ TimedatedTimezone & m_owner; - unsigned long m_properties_changed_id = 0; - unsigned long m_bus_watch_id = 0; - GDBusProxy *m_proxy = nullptr; + GDBusConnection *m_connection = nullptr; + unsigned long m_signal_subscription_id = 0; std::string m_filename; }; -- cgit v1.2.3 From 60f30453fb89d3769874af42a4f24c372bcad714 Mon Sep 17 00:00:00 2001 From: Lars Uebernickel Date: Wed, 9 Sep 2015 18:02:50 +0200 Subject: test-utils: don't pass NULL as gsettings string value NULL is not a valid string. Use the empty string instead. --- tests/test-utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-utils.cpp b/tests/test-utils.cpp index 97f07ed..f5c14d4 100644 --- a/tests/test-utils.cpp +++ b/tests/test-utils.cpp @@ -59,7 +59,7 @@ namespace const char* location; const char* expected_name; } beautify_timezone_test_cases[] = { - { "America/Chicago", nullptr, "Chicago" }, + { "America/Chicago", "", "Chicago" }, { "America/Chicago", "America/Chicago", "Chicago" }, { "America/Chicago", "America/Chigago Chicago", "Chicago" }, { "America/Chicago", "America/Chicago Oklahoma City", "Oklahoma City" }, -- cgit v1.2.3 From 046cd3a7dd67935eac5f8377996423c434f44419 Mon Sep 17 00:00:00 2001 From: Lars Uebernickel Date: Wed, 9 Sep 2015 18:16:41 +0200 Subject: glib-fixture: fail tests on unexpected warnings This requires specifying which warnings are expected to be thrown (only test-timezone-file needed this for now). However, only fail on warnings in the Indicator-Datetime log domain so that we don't fail on gstreamer (or other library) warnings for now. --- tests/CMakeLists.txt | 1 + tests/glib-fixture.h | 50 +++++++-------------------------------- tests/test-timezone-timedated.cpp | 3 ++- 3 files changed, 12 insertions(+), 42 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 032b84e..0302da9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -38,6 +38,7 @@ include_directories (${DBUSTEST_INCLUDE_DIRS}) add_definitions (-DSANDBOX="${CMAKE_CURRENT_BINARY_DIR}") +add_definitions (-DG_LOG_DOMAIN="Indicator-Datetime") function(add_test_by_name name) set (TEST_NAME ${name}) diff --git a/tests/glib-fixture.h b/tests/glib-fixture.h index f888c59..4d309e6 100644 --- a/tests/glib-fixture.h +++ b/tests/glib-fixture.h @@ -36,34 +36,6 @@ class GlibFixture : public ::testing::Test virtual ~GlibFixture() =default; - private: - - //GLogFunc realLogHandler; - - protected: - - std::map logCounts; - - void testLogCount(GLogLevelFlags log_level, int /*expected*/) - { -#if 0 - EXPECT_EQ(expected, logCounts[log_level]); -#endif - - logCounts.erase(log_level); - } - - private: - - static void default_log_handler(const gchar * log_domain, - GLogLevelFlags log_level, - const gchar * message, - gpointer self) - { - g_print("%s - %d - %s\n", log_domain, (int)log_level, message); - static_cast(self)->logCounts[log_level]++; - } - protected: virtual void SetUp() override @@ -72,34 +44,30 @@ class GlibFixture : public ::testing::Test loop = g_main_loop_new(nullptr, false); - //g_log_set_default_handler(default_log_handler, this); - // only use local, temporary settings g_assert(g_setenv("GSETTINGS_SCHEMA_DIR", SCHEMA_DIR, true)); g_assert(g_setenv("GSETTINGS_BACKEND", "memory", true)); g_debug("SCHEMA_DIR is %s", SCHEMA_DIR); + // fail on unexpected messages from this domain + g_log_set_fatal_mask(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING); + g_unsetenv("DISPLAY"); } virtual void TearDown() override { -#if 0 - // confirm there aren't any unexpected log messages - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_ERROR]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_CRITICAL]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_WARNING]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_MESSAGE]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_INFO]); -#endif - - // revert to glib's log handler - //g_log_set_default_handler(realLogHandler, this); + g_test_assert_expected_messages (); g_clear_pointer(&loop, g_main_loop_unref); } + void expectLogMessage (const gchar *domain, GLogLevelFlags level, const gchar *pattern) + { + g_test_expect_message (domain, level, pattern); + } + private: static gboolean diff --git a/tests/test-timezone-timedated.cpp b/tests/test-timezone-timedated.cpp index 5cfc311..7300649 100644 --- a/tests/test-timezone-timedated.cpp +++ b/tests/test-timezone-timedated.cpp @@ -69,8 +69,8 @@ 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); - testLogCount(G_LOG_LEVEL_WARNING, 1); } /** @@ -82,6 +82,7 @@ TEST_F(TimezoneFixture, DefaultValueNoFile) 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); ASSERT_EQ(expected_timezone, tz.timezone.get()); } -- cgit v1.2.3 From ec001e64b2225a3c80b7e89d9a570728fcbca830 Mon Sep 17 00:00:00 2001 From: Lars Uebernickel Date: Wed, 9 Sep 2015 18:16:41 +0200 Subject: state-fixture: use TestDBusFixture for system bus --- tests/state-fixture.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/state-fixture.h b/tests/state-fixture.h index e466a79..341df8b 100644 --- a/tests/state-fixture.h +++ b/tests/state-fixture.h @@ -20,7 +20,7 @@ #ifndef INDICATOR_DATETIME_TESTS_STATE_FIXTURE_H #define INDICATOR_DATETIME_TESTS_STATE_FIXTURE_H -#include "glib-fixture.h" +#include "test-dbus-fixture.h" #include "actions-mock.h" #include "state-mock.h" @@ -33,10 +33,10 @@ namespace datetime { **** ***/ -class StateFixture: public GlibFixture +class StateFixture: public TestDBusFixture { private: - typedef GlibFixture super; + typedef TestDBusFixture super; public: virtual ~StateFixture() =default; -- cgit v1.2.3