Merge lp:~laney/indicator-datetime/timedated-timezone-property into lp:indicator-datetime/15.10

Proposed by Iain Lane on 2015-09-01
Status: Rejected
Rejected by: Iain Lane on 2015-09-03
Proposed branch: lp:~laney/indicator-datetime/timedated-timezone-property
Merge into: lp:indicator-datetime/15.10
Diff against target: 1240 lines (+533/-351)
10 files modified
include/datetime/timezone-timedated.h (+10/-8)
include/datetime/timezones-live.h (+4/-4)
src/CMakeLists.txt (+2/-3)
src/main.cpp (+3/-3)
src/timezone-timedated.cpp (+155/-63)
src/timezones-live.cpp (+2/-3)
tests/CMakeLists.txt (+1/-1)
tests/test-live-actions.cpp (+21/-227)
tests/test-timezone-timedated.cpp (+34/-39)
tests/timedated-fixture.h (+301/-0)
To merge this branch: bzr merge lp:~laney/indicator-datetime/timedated-timezone-property
Reviewer Review Type Date Requested Status
Lars Karlitski (community) 2015-09-01 Needs Fixing on 2015-09-03
PS Jenkins bot (community) continuous-integration Approve on 2015-09-03
Indicator Applet Developers 2015-09-01 Pending
Review via email: mp+269732@code.launchpad.net

Description of the change

Use timedated's Timezone property to find out the current timezone and learn about changes to it

To post a comment you must log in.
Iain Lane (laney) wrote :

Some IRC chat reveals that a good option to read the initial state is to get this from /etc/timezone on disk directly, then use dbus after that.

424. By Iain Lane on 2015-09-03

Avoid nested GMainLoops by reading from the file on startup

Restore some tests for this functionality.

425. By Iain Lane on 2015-09-03

Make test-live-actions quit itself once the tz has been changed, the mock no longer does this

Iain Lane (laney) wrote :

Updated - please re-review.

Is taking a reference to the GDBusConnection sane? Without it the testsuite kills the connection and the AddMatch fails, which causes some criticals.

(process:12769): GLib-GIO-CRITICAL **: Error while sending AddMatch() message: The connection is closed

(process:12769): GLib-GIO-CRITICAL **: g_dbus_connection_call_finish_internal: assertion 'G_IS_DBUS_CONNECTION (connection)' failed

Maybe we don't care or there's a better way to ensure this can't fail?

Lars Karlitski (larsu) wrote :

Thanks for working on this!

It's causing some unnecessary bus traffic though:

 - watch org.freedesktop.timedate1
 - call GetProperties and fetch all properties of that object
 - subscribe to PropertiesChanged of all properties instead of just the one

Really all you need here is to add a match rule for PropertiesChanged for that *one* property you're interested in. Using g_dbus_connection_signal_subscribe() results in only one call to the daemon and you'll only get signals when the property you're interested in changes.

(By the way, the name_vanished() handler is called immediately when the name doesn't exist. Currently it's spewing some GObject warnings because you're trying to access the proxy without checking it for NULL. However, don't fix this if you stop watching the name anyway)

review: Needs Fixing
Iain Lane (laney) wrote :

There is no point in reviewing this.

I'm not working on this change. Get it in some other way.

Unmerged revisions

425. By Iain Lane on 2015-09-03

Make test-live-actions quit itself once the tz has been changed, the mock no longer does this

424. By Iain Lane on 2015-09-03

Avoid nested GMainLoops by reading from the file on startup

Restore some tests for this functionality.

423. By Iain Lane on 2015-09-01

Add some comments

422. By Iain Lane on 2015-09-01

Rename FileTimezone to TimedatedTimezone

421. By Iain Lane on 2015-09-01

Add a timeout so we can't hang forever

420. By Iain Lane on 2015-08-31

Use timedated's Timezone property instead of watching /etc/timezone

Still need to rename everything to not use "timezone-file"

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== renamed file 'include/datetime/timezone-file.h' => 'include/datetime/timezone-timedated.h'
2--- include/datetime/timezone-file.h 2014-09-13 20:45:23 +0000
3+++ include/datetime/timezone-timedated.h 2015-09-03 10:03:17 +0000
4@@ -17,8 +17,10 @@
5 * Charles Kerr <charles.kerr@canonical.com>
6 */
7
8-#ifndef INDICATOR_DATETIME_FILE_TIMEZONE_H
9-#define INDICATOR_DATETIME_FILE_TIMEZONE_H
10+#ifndef INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H
11+#define INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H
12+
13+#define DEFAULT_FILENAME "/etc/timezone"
14
15 #include <datetime/timezone.h> // base class
16
17@@ -31,11 +33,11 @@
18 /**
19 * \brief A #Timezone that gets its information from monitoring a file, such as /etc/timezone
20 */
21-class FileTimezone: public Timezone
22+class TimedatedTimezone: public Timezone
23 {
24 public:
25- FileTimezone(const std::string& filename);
26- ~FileTimezone();
27+ TimedatedTimezone(std::string filename = DEFAULT_FILENAME);
28+ ~TimedatedTimezone();
29
30 private:
31 class Impl;
32@@ -43,12 +45,12 @@
33 std::unique_ptr<Impl> impl;
34
35 // we have pointers in here, so disable copying
36- FileTimezone(const FileTimezone&) =delete;
37- FileTimezone& operator=(const FileTimezone&) =delete;
38+ TimedatedTimezone(const TimedatedTimezone&) =delete;
39+ TimedatedTimezone& operator=(const TimedatedTimezone&) =delete;
40 };
41
42 } // namespace datetime
43 } // namespace indicator
44 } // namespace unity
45
46-#endif // INDICATOR_DATETIME_FILE_TIMEZONE_H
47+#endif // INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H
48
49=== modified file 'include/datetime/timezones-live.h'
50--- include/datetime/timezones-live.h 2014-01-30 19:44:12 +0000
51+++ include/datetime/timezones-live.h 2015-09-03 10:03:17 +0000
52@@ -22,8 +22,8 @@
53
54 #include <datetime/settings.h>
55 #include <datetime/timezones.h>
56-#include <datetime/timezone-file.h>
57 #include <datetime/timezone-geoclue.h>
58+#include <datetime/timezone-timedated.h>
59
60 #include <memory> // shared_ptr<>
61
62@@ -32,19 +32,19 @@
63 namespace datetime {
64
65 /**
66- * \brief #Timezones object that uses a #FileTimezone and #GeoclueTimezone
67+ * \brief #Timezones object that uses a #TimedatedTimezone and #GeoclueTimezone
68 * to detect what timezone we're in
69 */
70 class LiveTimezones: public Timezones
71 {
72 public:
73- LiveTimezones(const std::shared_ptr<const Settings>& settings, const std::string& filename);
74+ LiveTimezones(const std::shared_ptr<const Settings>& settings);
75
76 private:
77 void update_geolocation();
78 void update_timezones();
79
80- FileTimezone m_file;
81+ TimedatedTimezone m_file;
82 std::shared_ptr<const Settings> m_settings;
83 std::shared_ptr<GeoclueTimezone> m_geo;
84 };
85
86=== modified file 'src/CMakeLists.txt'
87--- src/CMakeLists.txt 2015-01-21 20:25:52 +0000
88+++ src/CMakeLists.txt 2015-09-03 10:03:17 +0000
89@@ -1,8 +1,7 @@
90 set (SERVICE_LIB "indicatordatetimeservice")
91 set (SERVICE_EXEC "indicator-datetime-service")
92
93-add_definitions (-DTIMEZONE_FILE="/etc/timezone"
94- -DG_LOG_DOMAIN="Indicator-Datetime")
95+add_definitions (-DG_LOG_DOMAIN="Indicator-Datetime")
96
97 # handwritten sources
98 set (SERVICE_C_SOURCES
99@@ -34,9 +33,9 @@
100 settings-live.cpp
101 snap.cpp
102 sound.cpp
103- timezone-file.cpp
104 timezone-geoclue.cpp
105 timezones-live.cpp
106+ timezone-timedated.cpp
107 utils.c
108 wakeup-timer-mainloop.cpp
109 wakeup-timer-powerd.cpp)
110
111=== modified file 'src/main.cpp'
112--- src/main.cpp 2015-04-03 18:11:39 +0000
113+++ src/main.cpp 2015-09-03 10:03:17 +0000
114@@ -31,8 +31,8 @@
115 #include <datetime/settings-live.h>
116 #include <datetime/snap.h>
117 #include <datetime/state.h>
118-#include <datetime/timezone-file.h>
119 #include <datetime/timezones-live.h>
120+#include <datetime/timezone-timedated.h>
121 #include <datetime/wakeup-timer-powerd.h>
122 #include <notifications/notifications.h>
123
124@@ -68,7 +68,7 @@
125 {
126 // create the live objects
127 auto live_settings = std::make_shared<LiveSettings>();
128- auto live_timezones = std::make_shared<LiveTimezones>(live_settings, TIMEZONE_FILE);
129+ auto live_timezones = std::make_shared<LiveTimezones>(live_settings);
130 auto live_clock = std::make_shared<LiveClock>(timezone_);
131
132 // create a full-month planner currently pointing to the current month
133@@ -128,7 +128,7 @@
134 textdomain(GETTEXT_PACKAGE);
135
136 auto engine = create_engine();
137- auto timezone_ = std::make_shared<FileTimezone>(TIMEZONE_FILE);
138+ auto timezone_ = std::make_shared<TimedatedTimezone>();
139 auto state = create_state(engine, timezone_);
140 auto actions = std::make_shared<LiveActions>(state);
141 MenuFactory factory(actions, state);
142
143=== renamed file 'src/timezone-file.cpp' => 'src/timezone-timedated.cpp'
144--- src/timezone-file.cpp 2014-09-13 20:45:23 +0000
145+++ src/timezone-timedated.cpp 2015-09-03 10:03:17 +0000
146@@ -17,7 +17,7 @@
147 * Charles Kerr <charles.kerr@canonical.com>
148 */
149
150-#include <datetime/timezone-file.h>
151+#include <datetime/timezone-timedated.h>
152
153 #include <gio/gio.h>
154
155@@ -32,14 +32,16 @@
156 ****
157 ***/
158
159-class FileTimezone::Impl
160+class TimedatedTimezone::Impl
161 {
162 public:
163
164- Impl(FileTimezone& owner, const std::string& filename):
165- m_owner(owner)
166+ Impl(TimedatedTimezone& owner, std::string filename):
167+ m_owner(owner),
168+ m_filename(filename)
169 {
170- set_filename(filename);
171+ g_debug("Filename is '%s'", filename.c_str());
172+ monitor_timezone_property();
173 }
174
175 ~Impl()
176@@ -51,65 +53,152 @@
177
178 void clear()
179 {
180- if (m_monitor_handler_id)
181- g_signal_handler_disconnect(m_monitor, m_monitor_handler_id);
182-
183- g_clear_object (&m_monitor);
184-
185- m_filename.clear();
186- }
187-
188- void set_filename(const std::string& filename)
189- {
190- clear();
191-
192- auto tmp = realpath(filename.c_str(), nullptr);
193- if(tmp != nullptr)
194- {
195- m_filename = tmp;
196- free(tmp);
197- }
198- else
199- {
200- g_warning("Unable to resolve path '%s': %s", filename.c_str(), g_strerror(errno));
201- m_filename = filename; // better than nothing?
202- }
203-
204- auto file = g_file_new_for_path(m_filename.c_str());
205- GError * err = nullptr;
206- m_monitor = g_file_monitor_file(file, G_FILE_MONITOR_NONE, nullptr, &err);
207- g_object_unref(file);
208+ if (m_bus_watch_id)
209+ {
210+ g_bus_unwatch_name (m_bus_watch_id);
211+ m_bus_watch_id = 0;
212+ }
213+
214+ if (m_properties_changed_id)
215+ {
216+ g_signal_handler_disconnect(m_proxy, m_properties_changed_id);
217+ m_properties_changed_id = 0;
218+ }
219+
220+ g_clear_object(&m_proxy);
221+ }
222+
223+ static void on_properties_changed(GDBusProxy *proxy G_GNUC_UNUSED,
224+ GVariant *changed_properties /* a{sv} */,
225+ GStrv invalidated_properties G_GNUC_UNUSED,
226+ gpointer gself)
227+ {
228+ auto self = static_cast<Impl*>(gself);
229+ char *tz;
230+
231+ if (g_variant_lookup(changed_properties, "Timezone", "s", &tz, NULL))
232+ {
233+ g_debug("on_properties_changed: got timezone '%s'", tz);
234+ self->notify_timezone(tz);
235+ g_free (tz);
236+ }
237+ }
238+
239+ static void on_proxy_ready(GObject *object G_GNUC_UNUSED,
240+ GAsyncResult *res,
241+ gpointer gself)
242+ {
243+ auto self = static_cast<Impl*>(gself);
244+ GError *error = nullptr;
245+ self->m_proxy = g_dbus_proxy_new_finish(res, &error);
246+
247+ if (error)
248+ {
249+ g_warning ("Couldn't create proxy to read timezone: %s", error->message);
250+ goto out;
251+ }
252+
253+ /* Read the property */
254+ GVariant *prop;
255+ prop = g_dbus_proxy_get_cached_property(self->m_proxy, "Timezone");
256+
257+ if (!prop || !g_variant_is_of_type(prop, G_VARIANT_TYPE_STRING))
258+ {
259+ g_warning("Couldn't read the Timezone property, defaulting to Etc/Utc");
260+ self->notify_timezone("Etc/Utc");
261+ goto out;
262+ }
263+
264+ const gchar *tz;
265+ tz = g_variant_get_string(prop, nullptr);
266+
267+ self->notify_timezone(tz);
268+
269+ self->m_properties_changed_id = g_signal_connect(self->m_proxy,
270+ "g-properties-changed",
271+ (GCallback) on_properties_changed,
272+ gself);
273+
274+out:
275+ g_clear_pointer(&error, g_error_free);
276+ g_clear_pointer(&prop, g_variant_unref);
277+ }
278+
279+ static void on_name_appeared(GDBusConnection *connection,
280+ const gchar *name,
281+ const gchar *name_owner G_GNUC_UNUSED,
282+ gpointer gself G_GNUC_UNUSED)
283+ {
284+ g_debug ("timedate1 appeared");
285+ g_dbus_proxy_new(connection,
286+ G_DBUS_PROXY_FLAGS_NONE,
287+ NULL,
288+ name,
289+ "/org/freedesktop/timedate1",
290+ "org.freedesktop.timedate1",
291+ nullptr,
292+ on_proxy_ready,
293+ gself);
294+ }
295+
296+ static void on_name_vanished(GDBusConnection *connection G_GNUC_UNUSED,
297+ const gchar *name G_GNUC_UNUSED,
298+ gpointer gself)
299+ {
300+ auto self = static_cast<Impl*>(gself);
301+ g_debug ("timedate1 vanished");
302+
303+ g_signal_handler_disconnect(self->m_proxy,
304+ self->m_properties_changed_id);
305+ self->m_properties_changed_id = 0;
306+ g_clear_object(&self->m_proxy);
307+ g_clear_pointer(&self->m_proxy, g_main_loop_unref);
308+ }
309+
310+ void monitor_timezone_property()
311+ {
312+ GError *err = nullptr;
313+ GDBusConnection *conn;
314+
315+ /*
316+ * There is an unlikely race which happens if there is an activation
317+ * and timezone change before our match rule is added.
318+ */
319+ notify_timezone(get_timezone_from_file(m_filename));
320+
321+ /*
322+ * Make sure the bus is around at least until we add the match rules,
323+ * otherwise things (tests) are sad.
324+ */
325+ conn = g_bus_get_sync(G_BUS_TYPE_SYSTEM,
326+ nullptr,
327+ &err);
328+
329 if (err)
330 {
331- g_warning("%s Unable to monitor timezone file '%s': %s", G_STRLOC, TIMEZONE_FILE, err->message);
332+ g_warning("Couldn't get bus connection: '%s'", err->message);
333 g_error_free(err);
334- }
335- else
336- {
337- m_monitor_handler_id = g_signal_connect_swapped(m_monitor, "changed", G_CALLBACK(on_file_changed), this);
338- g_debug("%s Monitoring timezone file '%s'", G_STRLOC, m_filename.c_str());
339- }
340-
341- reload();
342- }
343-
344- static void on_file_changed(gpointer gself)
345- {
346- static_cast<Impl*>(gself)->reload();
347- }
348-
349- void reload()
350- {
351- const auto new_timezone = get_timezone_from_file(m_filename);
352-
353+ return;
354+ }
355+
356+ m_bus_watch_id = g_bus_watch_name_on_connection(conn,
357+ "org.freedesktop.timedate1",
358+ G_BUS_NAME_WATCHER_FLAGS_NONE,
359+ on_name_appeared,
360+ on_name_vanished,
361+ this,
362+ nullptr);
363+
364+ g_object_unref (conn);
365+ }
366+
367+ void notify_timezone(std::string new_timezone)
368+ {
369+ g_debug("notify_timezone '%s'", new_timezone.c_str());
370 if (!new_timezone.empty())
371 m_owner.timezone.set(new_timezone);
372 }
373
374- /***
375- ****
376- ***/
377-
378 std::string get_timezone_from_file(const std::string& filename)
379 {
380 GError * error;
381@@ -143,7 +232,9 @@
382 }
383
384 g_string_free(line, true);
385- }
386+ } else
387+ /* Default to UTC */
388+ ret = "Etc/Utc";
389
390 if (io_channel != nullptr)
391 {
392@@ -164,22 +255,23 @@
393 ****
394 ***/
395
396- FileTimezone & m_owner;
397+ TimedatedTimezone & m_owner;
398+ unsigned long m_properties_changed_id = 0;
399+ unsigned long m_bus_watch_id = 0;
400+ GDBusProxy *m_proxy = nullptr;
401 std::string m_filename;
402- GFileMonitor * m_monitor = nullptr;
403- unsigned long m_monitor_handler_id = 0;
404 };
405
406 /***
407 ****
408 ***/
409
410-FileTimezone::FileTimezone(const std::string& filename):
411+TimedatedTimezone::TimedatedTimezone(std::string filename):
412 impl(new Impl{*this, filename})
413 {
414 }
415
416-FileTimezone::~FileTimezone()
417+TimedatedTimezone::~TimedatedTimezone()
418 {
419 }
420
421
422=== modified file 'src/timezones-live.cpp'
423--- src/timezones-live.cpp 2014-01-30 19:44:12 +0000
424+++ src/timezones-live.cpp 2015-09-03 10:03:17 +0000
425@@ -25,9 +25,8 @@
426 namespace indicator {
427 namespace datetime {
428
429-LiveTimezones::LiveTimezones(const std::shared_ptr<const Settings>& settings,
430- const std::string& filename):
431- m_file(filename),
432+LiveTimezones::LiveTimezones(const std::shared_ptr<const Settings>& settings):
433+ m_file(),
434 m_settings(settings)
435 {
436 m_file.timezone.changed().connect([this](const std::string&){update_timezones();});
437
438=== modified file 'tests/CMakeLists.txt'
439--- tests/CMakeLists.txt 2015-07-22 12:00:11 +0000
440+++ tests/CMakeLists.txt 2015-09-03 10:03:17 +0000
441@@ -58,7 +58,7 @@
442 add_test_by_name(test-menus)
443 add_test_by_name(test-planner)
444 add_test_by_name(test-settings)
445-add_test_by_name(test-timezone-file)
446+add_test_by_name(test-timezone-timedated)
447 add_test_by_name(test-utils)
448
449 set (TEST_NAME manual-test-snap)
450
451=== modified file 'tests/test-live-actions.cpp'
452--- tests/test-live-actions.cpp 2015-03-31 23:54:04 +0000
453+++ tests/test-live-actions.cpp 2015-09-03 10:03:17 +0000
454@@ -17,228 +17,18 @@
455 * Charles Kerr <charles.kerr@canonical.com>
456 */
457
458-#include <datetime/actions-live.h>
459-
460-#include "state-mock.h"
461-#include "glib-fixture.h"
462-
463-/***
464-****
465-***/
466-
467-using namespace unity::indicator::datetime;
468-
469-class MockLiveActions: public LiveActions
470-{
471-public:
472- std::string last_cmd;
473- std::string last_url;
474- explicit MockLiveActions(const std::shared_ptr<State>& state_in): LiveActions(state_in) {}
475- ~MockLiveActions() {}
476-
477-protected:
478- void dispatch_url(const std::string& url) override { last_url = url; }
479- void execute_command(const std::string& cmd) override { last_cmd = cmd; }
480-};
481-
482-/***
483-****
484-***/
485-
486-using namespace unity::indicator::datetime;
487-
488-class LiveActionsFixture: public GlibFixture
489-{
490-private:
491-
492- typedef GlibFixture super;
493-
494- static void on_bus_acquired(GDBusConnection* conn,
495- const gchar* name,
496- gpointer gself)
497- {
498- auto self = static_cast<LiveActionsFixture*>(gself);
499- g_debug("bus acquired: %s, connection is %p", name, conn);
500-
501- // Set up a mock GSD.
502- // All it really does is wait for calls to GetDevice and
503- // returns the get_devices_retval variant
504- static const GDBusInterfaceVTable vtable = {
505- timedate1_handle_method_call,
506- nullptr, /* GetProperty */
507- nullptr, /* SetProperty */
508- };
509-
510- self->connection = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(conn)));
511-
512- GError* error = nullptr;
513- self->object_register_id = g_dbus_connection_register_object(
514- conn,
515- "/org/freedesktop/timedate1",
516- self->node_info->interfaces[0],
517- &vtable,
518- self,
519- nullptr,
520- &error);
521- g_assert_no_error(error);
522- }
523-
524- static void on_name_acquired(GDBusConnection* /*conn*/,
525- const gchar* /*name*/,
526- gpointer gself)
527- {
528- auto self = static_cast<LiveActionsFixture*>(gself);
529- self->name_acquired = true;
530- g_main_loop_quit(self->loop);
531- }
532-
533- static void on_name_lost(GDBusConnection* /*conn*/,
534- const gchar* /*name*/,
535- gpointer gself)
536- {
537- auto self = static_cast<LiveActionsFixture*>(gself);
538- self->name_acquired = false;
539- }
540-
541- static void on_bus_closed(GObject* /*object*/,
542- GAsyncResult* res,
543- gpointer gself)
544- {
545- auto self = static_cast<LiveActionsFixture*>(gself);
546- GError* err = nullptr;
547- g_dbus_connection_close_finish(self->connection, res, &err);
548- g_assert_no_error(err);
549- g_main_loop_quit(self->loop);
550- }
551-
552- static void
553- timedate1_handle_method_call(GDBusConnection * /*connection*/,
554- const gchar * /*sender*/,
555- const gchar * /*object_path*/,
556- const gchar * /*interface_name*/,
557- const gchar * method_name,
558- GVariant * parameters,
559- GDBusMethodInvocation * invocation,
560- gpointer gself)
561- {
562- g_assert(!g_strcmp0(method_name, "SetTimezone"));
563- g_assert(g_variant_is_of_type(parameters, G_VARIANT_TYPE_TUPLE));
564- g_assert(2 == g_variant_n_children(parameters));
565-
566- auto child = g_variant_get_child_value(parameters, 0);
567- g_assert(g_variant_is_of_type(child, G_VARIANT_TYPE_STRING));
568- auto self = static_cast<LiveActionsFixture*>(gself);
569- self->attempted_tzid = g_variant_get_string(child, nullptr);
570- g_variant_unref(child);
571-
572- g_dbus_method_invocation_return_value(invocation, nullptr);
573- g_main_loop_quit(self->loop);
574- }
575-
576-protected:
577-
578- std::shared_ptr<MockState> m_mock_state;
579- std::shared_ptr<State> m_state;
580- std::shared_ptr<MockLiveActions> m_live_actions;
581- std::shared_ptr<Actions> m_actions;
582-
583- bool name_acquired;
584- std::string attempted_tzid;
585-
586- GTestDBus* bus;
587- guint own_name;
588- GDBusConnection* connection;
589- GDBusNodeInfo* node_info;
590- int object_register_id;
591-
592- void SetUp()
593- {
594- super::SetUp();
595-
596- name_acquired = false;
597- attempted_tzid.clear();
598- connection = nullptr;
599- node_info = nullptr;
600- object_register_id = 0;
601- own_name = 0;
602-
603- // bring up the test bus
604- bus = g_test_dbus_new(G_TEST_DBUS_NONE);
605- g_test_dbus_up(bus);
606- const auto address = g_test_dbus_get_bus_address(bus);
607- g_setenv("DBUS_SYSTEM_BUS_ADDRESS", address, true);
608- g_setenv("DBUS_SESSION_BUS_ADDRESS", address, true);
609- g_debug("test_dbus's address is %s", address);
610-
611- // parse the org.freedesktop.timedate1 interface
612- const gchar introspection_xml[] =
613- "<node>"
614- " <interface name='org.freedesktop.timedate1'>"
615- " <method name='SetTimezone'>"
616- " <arg name='timezone' type='s' direction='in'/>"
617- " <arg name='user_interaction' type='b' direction='in'/>"
618- " </method>"
619- " </interface>"
620- "</node>";
621- node_info = g_dbus_node_info_new_for_xml(introspection_xml, nullptr);
622- ASSERT_TRUE(node_info != nullptr);
623- ASSERT_TRUE(node_info->interfaces != nullptr);
624- ASSERT_TRUE(node_info->interfaces[0] != nullptr);
625- ASSERT_TRUE(node_info->interfaces[1] == nullptr);
626- ASSERT_STREQ("org.freedesktop.timedate1", node_info->interfaces[0]->name);
627-
628- // own the bus
629- own_name = g_bus_own_name(G_BUS_TYPE_SYSTEM,
630- "org.freedesktop.timedate1",
631- G_BUS_NAME_OWNER_FLAGS_NONE,
632- on_bus_acquired, on_name_acquired, on_name_lost,
633- this, nullptr);
634- ASSERT_TRUE(object_register_id == 0);
635- ASSERT_FALSE(name_acquired);
636- ASSERT_TRUE(connection == nullptr);
637- g_main_loop_run(loop);
638- ASSERT_TRUE(object_register_id != 0);
639- ASSERT_TRUE(name_acquired);
640- ASSERT_TRUE(G_IS_DBUS_CONNECTION(connection));
641-
642- // create the State and Actions
643- m_mock_state.reset(new MockState);
644- m_mock_state->settings.reset(new Settings);
645- m_state = std::dynamic_pointer_cast<State>(m_mock_state);
646- m_live_actions.reset(new MockLiveActions(m_state));
647- m_actions = std::dynamic_pointer_cast<Actions>(m_live_actions);
648- }
649-
650- void TearDown()
651- {
652- m_actions.reset();
653- m_live_actions.reset();
654- m_state.reset();
655- m_mock_state.reset();
656-
657- g_dbus_connection_unregister_object(connection, object_register_id);
658- g_dbus_node_info_unref(node_info);
659- g_bus_unown_name(own_name);
660- g_dbus_connection_close(connection, nullptr, on_bus_closed, this);
661- g_main_loop_run(loop);
662- g_clear_object(&connection);
663- g_test_dbus_down(bus);
664- g_clear_object(&bus);
665-
666- super::TearDown();
667- }
668-};
669-
670-/***
671-****
672-***/
673-
674-TEST_F(LiveActionsFixture, HelloWorld)
675+#include "timedated-fixture.h"
676+
677+/***
678+****
679+***/
680+
681+TEST_F(TimedateFixture, HelloWorld)
682 {
683 EXPECT_TRUE(true);
684 }
685
686-TEST_F(LiveActionsFixture, SetLocation)
687+TEST_F(TimedateFixture, SetLocation)
688 {
689 const std::string tzid = "America/Chicago";
690 const std::string name = "Oklahoma City";
691@@ -247,6 +37,10 @@
692 EXPECT_NE(expected, m_state->settings->timezone_name.get());
693
694 m_actions->set_location(tzid, name);
695+ m_state->settings->timezone_name.changed().connect(
696+ [this](const std::string&){
697+ g_main_loop_quit(loop);
698+ });
699 g_main_loop_run(loop);
700 EXPECT_EQ(attempted_tzid, tzid);
701 wait_msec();
702@@ -258,14 +52,14 @@
703 ****
704 ***/
705
706-TEST_F(LiveActionsFixture, DesktopOpenAlarmApp)
707+TEST_F(TimedateFixture, DesktopOpenAlarmApp)
708 {
709 m_actions->desktop_open_alarm_app();
710 const std::string expected = "evolution -c calendar";
711 EXPECT_EQ(expected, m_live_actions->last_cmd);
712 }
713
714-TEST_F(LiveActionsFixture, DesktopOpenAppointment)
715+TEST_F(TimedateFixture, DesktopOpenAppointment)
716 {
717 Appointment a;
718 a.uid = "some-uid";
719@@ -275,14 +69,14 @@
720 EXPECT_NE(m_live_actions->last_cmd.find(expected_substr), std::string::npos);
721 }
722
723-TEST_F(LiveActionsFixture, DesktopOpenCalendarApp)
724+TEST_F(TimedateFixture, DesktopOpenCalendarApp)
725 {
726 m_actions->desktop_open_calendar_app(DateTime::NowLocal());
727 const std::string expected_substr = "evolution \"calendar:///?startdate=";
728 EXPECT_NE(m_live_actions->last_cmd.find(expected_substr), std::string::npos);
729 }
730
731-TEST_F(LiveActionsFixture, DesktopOpenSettingsApp)
732+TEST_F(TimedateFixture, DesktopOpenSettingsApp)
733 {
734 m_actions->desktop_open_settings_app();
735 const std::string expected_substr = "control-center";
736@@ -300,13 +94,13 @@
737 const std::string calendar_app_url = "appid://com.ubuntu.calendar/calendar/current-user-version";
738 }
739
740-TEST_F(LiveActionsFixture, PhoneOpenAlarmApp)
741+TEST_F(TimedateFixture, PhoneOpenAlarmApp)
742 {
743 m_actions->phone_open_alarm_app();
744 EXPECT_EQ(clock_app_url, m_live_actions->last_url);
745 }
746
747-TEST_F(LiveActionsFixture, PhoneOpenAppointment)
748+TEST_F(TimedateFixture, PhoneOpenAppointment)
749 {
750 Appointment a;
751
752@@ -321,14 +115,14 @@
753 EXPECT_EQ(clock_app_url, m_live_actions->last_url);
754 }
755
756-TEST_F(LiveActionsFixture, PhoneOpenCalendarApp)
757+TEST_F(TimedateFixture, PhoneOpenCalendarApp)
758 {
759 m_actions->phone_open_calendar_app(DateTime::NowLocal());
760 const std::string expected = "appid://com.ubuntu.calendar/calendar/current-user-version";
761 EXPECT_EQ(expected, m_live_actions->last_url);
762 }
763
764-TEST_F(LiveActionsFixture, PhoneOpenSettingsApp)
765+TEST_F(TimedateFixture, PhoneOpenSettingsApp)
766 {
767 m_actions->phone_open_settings_app();
768 const std::string expected = "settings:///system/time-date";
769@@ -339,7 +133,7 @@
770 ****
771 ***/
772
773-TEST_F(LiveActionsFixture, CalendarState)
774+TEST_F(TimedateFixture, CalendarState)
775 {
776 // init the clock
777 auto now = DateTime::Local(2014, 1, 1, 0, 0, 0);
778
779=== renamed file 'tests/test-timezone-file.cpp' => 'tests/test-timezone-timedated.cpp'
780--- tests/test-timezone-file.cpp 2014-09-17 16:51:51 +0000
781+++ tests/test-timezone-timedated.cpp 2015-09-03 10:03:17 +0000
782@@ -18,25 +18,11 @@
783 * with this program. If not, see <http://www.gnu.org/licenses/>.
784 */
785
786-#include "glib-fixture.h"
787-
788-#include <datetime/timezone-file.h>
789-
790-//#include <condition_variable>
791-//#include <mutex>
792-//#include <queue>
793-//#include <string>
794-//#include <thread>
795-//#include <iostream>
796-//#include <istream>
797-//#include <fstream>
798-
799-#include <cstdio> // fopen()
800-//#include <sys/stat.h> // chmod()
801-#include <unistd.h> // sync()
802-
803-using unity::indicator::datetime::FileTimezone;
804-
805+#include "timedated-fixture.h"
806+
807+#include <datetime/timezone-timedated.h>
808+
809+using unity::indicator::datetime::TimedatedTimezone;
810
811 /***
812 ****
813@@ -44,11 +30,11 @@
814
815 #define TIMEZONE_FILE (SANDBOX"/timezone")
816
817-class TimezoneFixture: public GlibFixture
818+class TimezoneFixture: public TimedateFixture
819 {
820 private:
821
822- typedef GlibFixture super;
823+ typedef TimedateFixture super;
824
825 protected:
826
827@@ -67,6 +53,7 @@
828 /* convenience func to set the timezone file */
829 void set_file(const std::string& text)
830 {
831+ g_debug("set_file %s %s", TIMEZONE_FILE, text.c_str());
832 auto fp = fopen(TIMEZONE_FILE, "w+");
833 fprintf(fp, "%s\n", text.c_str());
834 fclose(fp);
835@@ -74,46 +61,56 @@
836 }
837 };
838
839-
840 /**
841- * Test that timezone-file warns, but doesn't crash, if the timezone file doesn't exist
842+ * Test that timezone-timedated warns, but doesn't crash, if the timezone file doesn't exist
843 */
844 TEST_F(TimezoneFixture, NoFile)
845 {
846 remove(TIMEZONE_FILE);
847 ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS));
848
849- FileTimezone tz(TIMEZONE_FILE);
850+ TimedatedTimezone tz(TIMEZONE_FILE);
851 testLogCount(G_LOG_LEVEL_WARNING, 1);
852 }
853
854-
855-/**
856- * Test that timezone-file picks up the initial value
857+/**
858+ * Test that timezone-timedated gives a default of UTC if the file doesn't exist
859+ */
860+TEST_F(TimezoneFixture, DefaultValueNoFile)
861+{
862+ const std::string expected_timezone = "Etc/Utc";
863+ remove(TIMEZONE_FILE);
864+ ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS));
865+
866+ TimedatedTimezone tz(TIMEZONE_FILE);
867+ ASSERT_EQ(expected_timezone, tz.timezone.get());
868+}
869+
870+/**
871+ * Test that timezone-timedated picks up the initial value
872 */
873 TEST_F(TimezoneFixture, InitialValue)
874 {
875 const std::string expected_timezone = "America/Chicago";
876 set_file(expected_timezone);
877- FileTimezone tz(TIMEZONE_FILE);
878- ASSERT_EQ(expected_timezone, tz.timezone.get());
879+ TimedatedTimezone tz(TIMEZONE_FILE);
880 }
881
882-
883 /**
884- * Test that clearing the timezone results in an empty string
885+ * Test that changing the tz after we are running works.
886 */
887 TEST_F(TimezoneFixture, ChangedValue)
888 {
889 const std::string initial_timezone = "America/Chicago";
890 const std::string changed_timezone = "America/New_York";
891+
892 set_file(initial_timezone);
893
894- FileTimezone tz(TIMEZONE_FILE);
895+ TimedatedTimezone tz(TIMEZONE_FILE);
896 ASSERT_EQ(initial_timezone, tz.timezone.get());
897
898 bool changed = false;
899- auto connection = tz.timezone.changed().connect(
900+ tz.timezone.changed().connect(
901 [&changed, this](const std::string& s){
902 g_message("timezone changed to %s", s.c_str());
903 changed = true;
904@@ -121,10 +118,9 @@
905 });
906
907 g_idle_add([](gpointer gself){
908- static_cast<TimezoneFixture*>(gself)->set_file("America/New_York");
909- // static_cast<FileTimezone*>(gtz)->timezone.set("America/New_York");
910+ static_cast<TimedateFixture*>(gself)->set_timezone("America/New_York");
911 return G_SOURCE_REMOVE;
912- }, this);//&tz);
913+ }, this);
914
915 g_main_loop_run(loop);
916
917@@ -132,15 +128,14 @@
918 ASSERT_EQ(changed_timezone, tz.timezone.get());
919 }
920
921-
922 /**
923- * Test that timezone-file picks up the initial value
924+ * Test that timezone-timedated picks up the initial value
925 */
926 TEST_F(TimezoneFixture, IgnoreComments)
927 {
928 const std::string comment = "# Created by cloud-init v. 0.7.5 on Thu, 24 Apr 2014 14:03:29 +0000";
929 const std::string expected_timezone = "Europe/Berlin";
930 set_file(comment + "\n" + expected_timezone);
931- FileTimezone tz(TIMEZONE_FILE);
932+ TimedatedTimezone tz(TIMEZONE_FILE);
933 ASSERT_EQ(expected_timezone, tz.timezone.get());
934 }
935
936=== added file 'tests/timedated-fixture.h'
937--- tests/timedated-fixture.h 1970-01-01 00:00:00 +0000
938+++ tests/timedated-fixture.h 2015-09-03 10:03:17 +0000
939@@ -0,0 +1,301 @@
940+/*
941+ * Copyright 2013 Canonical Ltd.
942+ *
943+ * This program is free software: you can redistribute it and/or modify it
944+ * under the terms of the GNU General Public License version 3, as published
945+ * by the Free Software Foundation.
946+ *
947+ * This program is distributed in the hope that it will be useful, but
948+ * WITHOUT ANY WARRANTY; without even the implied warranties of
949+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
950+ * PURPOSE. See the GNU General Public License for more details.
951+ *
952+ * You should have received a copy of the GNU General Public License along
953+ * with this program. If not, see <http://www.gnu.org/licenses/>.
954+ *
955+ * Authors:
956+ * Charles Kerr <charles.kerr@canonical.com>
957+ */
958+
959+#ifndef INDICATOR_DATETIME_TESTS_TIMEDATED_FIXTURE_H
960+#define INDICATOR_DATETIME_TESTS_TIMEDATED_FIXTURE_H
961+
962+#include <datetime/actions-live.h>
963+
964+#include "state-mock.h"
965+#include "glib-fixture.h"
966+
967+using namespace unity::indicator::datetime;
968+
969+class MockLiveActions: public LiveActions
970+{
971+public:
972+ std::string last_cmd;
973+ std::string last_url;
974+ explicit MockLiveActions(const std::shared_ptr<State>& state_in): LiveActions(state_in) {}
975+ ~MockLiveActions() {}
976+
977+protected:
978+ void dispatch_url(const std::string& url) override { last_url = url; }
979+ void execute_command(const std::string& cmd) override { last_cmd = cmd; }
980+};
981+
982+/***
983+****
984+***/
985+
986+using namespace unity::indicator::datetime;
987+
988+class TimedateFixture: public GlibFixture
989+{
990+private:
991+
992+ typedef GlibFixture super;
993+
994+ static GVariant * timedate1_get_properties (GDBusConnection * /*connection*/ ,
995+ const gchar * /*sender*/,
996+ const gchar * /*object_path*/,
997+ const gchar * /*interface_name*/,
998+ const gchar *property_name,
999+ GError ** /*error*/,
1000+ gpointer gself)
1001+
1002+ {
1003+ auto self = static_cast<TimedateFixture*>(gself);
1004+ g_debug("get_properties called");
1005+ if (g_strcmp0(property_name, "Timezone") == 0)
1006+ {
1007+ g_debug("timezone requested, giving '%s'",
1008+ self->attempted_tzid.c_str());
1009+ return g_variant_new_string(self->attempted_tzid.c_str());
1010+ }
1011+ return nullptr;
1012+ }
1013+
1014+
1015+ static void on_bus_acquired(GDBusConnection* conn,
1016+ const gchar* name,
1017+ gpointer gself)
1018+ {
1019+ auto self = static_cast<TimedateFixture*>(gself);
1020+ g_debug("bus acquired: %s, connection is %p", name, conn);
1021+
1022+ /* Set up a fake timedated which handles setting and getting the
1023+ ** timezone
1024+ */
1025+ static const GDBusInterfaceVTable vtable = {
1026+ timedate1_handle_method_call,
1027+ timedate1_get_properties, /* GetProperty */
1028+ nullptr, /* SetProperty */
1029+ };
1030+
1031+ self->connection = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(conn)));
1032+
1033+ GError* error = nullptr;
1034+ self->object_register_id = g_dbus_connection_register_object(
1035+ conn,
1036+ "/org/freedesktop/timedate1",
1037+ self->node_info->interfaces[0],
1038+ &vtable,
1039+ self,
1040+ nullptr,
1041+ &error);
1042+ g_assert_no_error(error);
1043+ }
1044+
1045+ static void on_name_acquired(GDBusConnection* conn,
1046+ const gchar* name,
1047+ gpointer gself)
1048+ {
1049+ g_debug("on_name_acquired");
1050+ auto self = static_cast<TimedateFixture*>(gself);
1051+ self->name_acquired = true;
1052+ self->proxy = g_dbus_proxy_new_sync(conn,
1053+ G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES,
1054+ nullptr,
1055+ name,
1056+ "/org/freedesktop/timedate1",
1057+ "org.freedesktop.timedate1",
1058+ nullptr,
1059+ nullptr);
1060+ g_main_loop_quit(self->loop);
1061+ }
1062+
1063+ static void on_name_lost(GDBusConnection* /*conn*/,
1064+ const gchar* /*name*/,
1065+ gpointer gself)
1066+ {
1067+ g_debug("on_name_lost");
1068+ auto self = static_cast<TimedateFixture*>(gself);
1069+ self->name_acquired = false;
1070+ }
1071+
1072+ static void on_bus_closed(GObject* /*object*/,
1073+ GAsyncResult* res,
1074+ gpointer gself)
1075+ {
1076+ g_debug("on_bus_closed");
1077+ auto self = static_cast<TimedateFixture*>(gself);
1078+ GError* err = nullptr;
1079+ g_dbus_connection_close_finish(self->connection, res, &err);
1080+ g_assert_no_error(err);
1081+ g_main_loop_quit(self->loop);
1082+ }
1083+
1084+ static void
1085+ timedate1_handle_method_call(GDBusConnection * connection,
1086+ const gchar * /*sender*/,
1087+ const gchar * object_path,
1088+ const gchar * interface_name,
1089+ const gchar * method_name,
1090+ GVariant * parameters,
1091+ GDBusMethodInvocation * invocation,
1092+ gpointer gself)
1093+ {
1094+ g_assert(!g_strcmp0(method_name, "SetTimezone"));
1095+ g_assert(g_variant_is_of_type(parameters, G_VARIANT_TYPE_TUPLE));
1096+ g_assert(2 == g_variant_n_children(parameters));
1097+
1098+ auto child = g_variant_get_child_value(parameters, 0);
1099+ g_assert(g_variant_is_of_type(child, G_VARIANT_TYPE_STRING));
1100+ auto self = static_cast<TimedateFixture*>(gself);
1101+ self->attempted_tzid = g_variant_get_string(child, nullptr);
1102+ g_debug("set tz (dbus side): '%s'", self->attempted_tzid.c_str());
1103+ g_dbus_method_invocation_return_value(invocation, nullptr);
1104+
1105+ /* Send PropertiesChanged */
1106+ GError * local_error = nullptr;
1107+ auto builder = g_variant_builder_new (G_VARIANT_TYPE_ARRAY);
1108+ g_variant_builder_add (builder,
1109+ "{sv}",
1110+ "Timezone",
1111+ g_variant_new_string(
1112+ self->attempted_tzid.c_str()));
1113+ g_dbus_connection_emit_signal (connection,
1114+ NULL,
1115+ object_path,
1116+ "org.freedesktop.DBus.Properties",
1117+ "PropertiesChanged",
1118+ g_variant_new ("(sa{sv}as)",
1119+ interface_name,
1120+ builder,
1121+ NULL),
1122+ &local_error);
1123+ g_assert_no_error (local_error);
1124+ g_variant_unref(child);
1125+ }
1126+
1127+protected:
1128+
1129+ std::shared_ptr<MockState> m_mock_state;
1130+ std::shared_ptr<State> m_state;
1131+ std::shared_ptr<MockLiveActions> m_live_actions;
1132+ std::shared_ptr<Actions> m_actions;
1133+
1134+ bool name_acquired;
1135+ std::string attempted_tzid;
1136+
1137+ GTestDBus* bus;
1138+ guint own_name;
1139+ GDBusConnection* connection;
1140+ GDBusNodeInfo* node_info;
1141+ int object_register_id;
1142+ GDBusProxy *proxy;
1143+
1144+ void SetUp()
1145+ {
1146+ super::SetUp();
1147+ g_debug("SetUp");
1148+
1149+ name_acquired = false;
1150+ attempted_tzid.clear();
1151+ connection = nullptr;
1152+ node_info = nullptr;
1153+ object_register_id = 0;
1154+ own_name = 0;
1155+ proxy = nullptr;
1156+
1157+ // bring up the test bus
1158+ bus = g_test_dbus_new(G_TEST_DBUS_NONE);
1159+ g_test_dbus_up(bus);
1160+ const auto address = g_test_dbus_get_bus_address(bus);
1161+ g_setenv("DBUS_SYSTEM_BUS_ADDRESS", address, true);
1162+ g_setenv("DBUS_SESSION_BUS_ADDRESS", address, true);
1163+ g_debug("test_dbus's address is %s", address);
1164+
1165+ // parse the org.freedesktop.timedate1 interface
1166+ const gchar introspection_xml[] =
1167+ "<node>"
1168+ " <interface name='org.freedesktop.timedate1'>"
1169+ " <property name='Timezone' type='s' access='read' />"
1170+ " <method name='SetTimezone'>"
1171+ " <arg name='timezone' type='s' direction='in'/>"
1172+ " <arg name='user_interaction' type='b' direction='in'/>"
1173+ " </method>"
1174+ " </interface>"
1175+ "</node>";
1176+ node_info = g_dbus_node_info_new_for_xml(introspection_xml, nullptr);
1177+ ASSERT_TRUE(node_info != nullptr);
1178+ ASSERT_TRUE(node_info->interfaces != nullptr);
1179+ ASSERT_TRUE(node_info->interfaces[0] != nullptr);
1180+ ASSERT_TRUE(node_info->interfaces[1] == nullptr);
1181+ ASSERT_STREQ("org.freedesktop.timedate1", node_info->interfaces[0]->name);
1182+
1183+ // own the bus
1184+ own_name = g_bus_own_name(G_BUS_TYPE_SYSTEM,
1185+ "org.freedesktop.timedate1",
1186+ G_BUS_NAME_OWNER_FLAGS_NONE,
1187+ on_bus_acquired, on_name_acquired, on_name_lost,
1188+ this, nullptr);
1189+ ASSERT_TRUE(object_register_id == 0);
1190+ ASSERT_FALSE(name_acquired);
1191+ ASSERT_TRUE(connection == nullptr);
1192+ g_main_loop_run(loop);
1193+ ASSERT_TRUE(object_register_id != 0);
1194+ ASSERT_TRUE(name_acquired);
1195+ ASSERT_TRUE(G_IS_DBUS_CONNECTION(connection));
1196+
1197+ // create the State and Actions
1198+ m_mock_state.reset(new MockState);
1199+ m_mock_state->settings.reset(new Settings);
1200+ m_state = std::dynamic_pointer_cast<State>(m_mock_state);
1201+ m_live_actions.reset(new MockLiveActions(m_state));
1202+ m_actions = std::dynamic_pointer_cast<Actions>(m_live_actions);
1203+ }
1204+
1205+ void TearDown()
1206+ {
1207+ g_debug("TearDown");
1208+ m_actions.reset();
1209+ m_live_actions.reset();
1210+ m_state.reset();
1211+ m_mock_state.reset();
1212+ g_dbus_connection_unregister_object(connection, object_register_id);
1213+ g_object_unref(proxy);
1214+ g_dbus_node_info_unref(node_info);
1215+ g_bus_unown_name(own_name);
1216+ g_dbus_connection_close(connection, nullptr, on_bus_closed, this);
1217+ g_main_loop_run(loop);
1218+ g_clear_object(&connection);
1219+ g_test_dbus_down(bus);
1220+ g_clear_object(&bus);
1221+
1222+ super::TearDown();
1223+ }
1224+public:
1225+ void set_timezone(std::string tz)
1226+ {
1227+ g_debug("set_timezone: '%s'", tz.c_str());
1228+ g_dbus_proxy_call_sync(proxy,
1229+ "SetTimezone",
1230+ g_variant_new("(sb)",
1231+ tz.c_str(),
1232+ FALSE),
1233+ G_DBUS_CALL_FLAGS_NONE,
1234+ 500,
1235+ nullptr,
1236+ nullptr);
1237+ }
1238+};
1239+
1240+#endif

Subscribers

People subscribed via source and target branches