[ofono] Fixed signal emission, reworked D-Bus client list. JB#50816

1. Exposed D-Bus clients list to plugin as ofono_dbus_clients
2. Signal has to be properly declared, otherwise it's not emitted
3. Added missing unit tests

ofono/.gitignore

@@ -45,6 +45,7 @@ unit/test-caif
 unit/test-stkutil
 unit/test-cdmasms
 unit/test-dbus-access
+unit/test-dbus-clients
 unit/test-dbus-queue
 unit/test-gprs-filter
 unit/test-ril_config

ofono/Makefile.am

@@ -26,7 +26,7 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/sms-filter.h include/gprs-filter.h \
 			include/voicecall-filter.h include/dbus-access.h \
 			include/ril-constants.h include/ril-transport.h \
-			include/watch.h gdbus/gdbus.h \
+			include/watch.h gdbus/gdbus.h include/dbus-clients.h \
 			include/netmon.h include/lte.h include/ims.h \
 			include/storage.h
 
@@ -126,8 +126,7 @@ builtin_sources += plugins/sailfish_manager/sailfish_cell_info.c \
 			plugins/sailfish_manager/sailfish_manager.c \
 			plugins/sailfish_manager/sailfish_manager_dbus.c \
 			plugins/sailfish_manager/sailfish_sim_info.c \
-			plugins/sailfish_manager/sailfish_sim_info_dbus.c \
-			plugins/sailfish_manager/sailfish_dbus_clients.c
+			plugins/sailfish_manager/sailfish_sim_info_dbus.c
 endif
 
 
@@ -776,7 +775,8 @@ src_ofonod_SOURCES = $(builtin_sources) $(gatchat_sources) src/ofono.ver \
 			src/handsfree-audio.c src/bluetooth.h \
 			src/sim-mnclength.c src/voicecallagent.c \
 			src/sms-filter.c src/gprs-filter.c \
-			src/dbus-queue.c src/dbus-access.c src/config.c \
+			src/dbus-clients.c src/dbus-queue.c \
+			src/dbus-access.c src/config.c \
 			src/voicecall-filter.c src/ril-transport.c \
 			src/hfp.h src/siri.c src/watchlist.c \
 			src/netmon.c src/lte.c src/ims.c \
@@ -980,10 +980,9 @@ unit_tests += unit/test-sailfish_cell_info
 unit_test_sailfish_cell_info_dbus_SOURCES = unit/test-dbus.c \
 			unit/test-sailfish_cell_info_dbus.c \
 			unit/fake_sailfish_cell_info.c \
-			unit/fake_sailfish_dbus_clients.c \
 			plugins/sailfish_manager/sailfish_cell_info.c \
 			plugins/sailfish_manager/sailfish_cell_info_dbus.c \
-			gdbus/object.c \
+			gdbus/object.c src/dbus-clients.c \
 			src/dbus.c src/log.c
 unit_test_sailfish_cell_info_dbus_CFLAGS = $(AM_CFLAGS) $(COVERAGE_OPT) \
 			@DBUS_GLIB_CFLAGS@
@@ -1166,6 +1165,14 @@ unit_test_caif_CFLAGS = $(COVERAGE_OPT) $(AM_CFLAGS)
 unit_test_caif_LDADD = @GLIB_LIBS@
 unit_objects += $(unit_test_caif_OBJECTS)
 
+unit_test_dbus_clients_SOURCES = unit/test-dbus-clients.c unit/test-dbus.c \
+				src/dbus-clients.c gdbus/object.c \
+				src/dbus.c src/log.c
+unit_test_dbus_clients_CFLAGS =  @DBUS_GLIB_CFLAGS@ $(COVERAGE_OPT) $(AM_CFLAGS)
+unit_test_dbus_clients_LDADD = @DBUS_GLIB_LIBS@ @GLIB_LIBS@ -ldl
+unit_objects += $(unit_test_dbus_clients_OBJECTS)
+unit_tests += unit/test-dbus-clients
+
 unit_test_dbus_queue_SOURCES = unit/test-dbus-queue.c unit/test-dbus.c \
 				src/dbus-queue.c gdbus/object.c \
 				src/dbus.c src/log.c

ofono/include/dbus-clients.h

@@ -0,0 +1,55 @@
+/*
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2021 Jolla Ltd.
+ *  Copyright (C) 2021 Open Mobile Platform LLC.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 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 warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#ifndef OFONO_DBUS_CLIENTS_H
+#define OFONO_DBUS_CLIENTS_H
+
+#include <ofono/types.h>
+#include <ofono/dbus.h>
+
+/* Since mer/1.23+git31 */
+
+struct ofono_dbus_clients;
+
+typedef void (*ofono_dbus_clients_notify_func)(const char *name,
+							void *user_data);
+
+struct ofono_dbus_clients *ofono_dbus_clients_new(DBusConnection *conn,
+		ofono_dbus_clients_notify_func notify, void *user_data);
+void ofono_dbus_clients_free(struct ofono_dbus_clients *clients);
+
+unsigned int ofono_dbus_clients_count(struct ofono_dbus_clients *clients);
+
+ofono_bool_t ofono_dbus_clients_add(struct ofono_dbus_clients *clients,
+							const char *name);
+ofono_bool_t ofono_dbus_clients_remove(struct ofono_dbus_clients *clients,
+							const char *name);
+
+void ofono_dbus_clients_signal(struct ofono_dbus_clients *clients,
+							DBusMessage *signal);
+void ofono_dbus_clients_signal_property_changed(struct ofono_dbus_clients *dc,
+		const char *path, const char *interface, const char *name,
+		int type, const void *value);
+
+#endif /* OFONO_DBUS_CLIENTS_H */
+
+/*
+ * Local Variables:
+ * mode: C
+ * c-basic-offset: 8
+ * indent-tabs-mode: t
+ * End:
+ */

ofono/include/dbus.h

@@ -3,7 +3,7 @@
  *  oFono - Open Telephony stack for Linux
  *
  *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
- *  Copyright (C) 2013-2016 Jolla Ltd.
+ *  Copyright (C) 2013-2021  Jolla Ltd.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -14,10 +14,6 @@
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
  *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
  */
 
 #ifndef __OFONO_DBUS_H
@@ -83,6 +79,8 @@ extern "C" {
 					DBUS_TYPE_VARIANT_AS_STRING \
 					DBUS_DICT_ENTRY_END_CHAR_AS_STRING
 
+#define OFONO_ERROR_INTERFACE "org.ofono.Error"
+
 DBusConnection *ofono_dbus_get_connection(void);
 
 void ofono_dbus_dict_append(DBusMessageIter *dict, const char *key, int type,
@@ -110,6 +108,11 @@ int ofono_dbus_signal_dict_property_changed(DBusConnection *conn,
 						const char *name, int type,
 						const void *value);
 
+/* Since mer/1.23+git31 */
+DBusMessage *ofono_dbus_signal_new_property_changed(const char *path,
+					const char *interface,
+					const char *name,
+					int type, const void *value);
 #ifdef __cplusplus
 }
 #endif

ofono/plugins/sailfish_manager/sailfish_cell_info_dbus.c

@@ -1,7 +1,7 @@
 /*
  *  oFono - Open Source Telephony - RIL-based devices
  *
- *  Copyright (C) 2016-2018 Jolla Ltd.
+ *  Copyright (C) 2016-2021 Jolla Ltd.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -15,11 +15,10 @@
 
 #include "sailfish_cell_info_dbus.h"
 #include "sailfish_cell_info.h"
-#include "sailfish_dbus_clients.h"
 
-#include <ofono.h>
 #include <ofono/modem.h>
 #include <ofono/dbus.h>
+#include <ofono/dbus-clients.h>
 #include <ofono/log.h>
 
 #include <gdbus.h>
@@ -37,7 +36,7 @@ struct sailfish_cell_info_dbus {
 	gulong handler_id;
 	guint next_cell_id;
 	GSList *entries;
-	struct sailfish_dbus_clients *clients;
+	struct ofono_dbus_clients *clients;
 };
 
 #define CELL_INFO_DBUS_INTERFACE            "org.nemomobile.ofono.CellInfo"
@@ -326,21 +325,24 @@ static void sailfish_cell_info_dbus_emit_path_list
 		(struct sailfish_cell_info_dbus *dbus, const char *name,
 							GPtrArray *list)
 {
-	guint i;
-	DBusMessageIter it, array;
-	DBusMessage *signal = dbus_message_new_signal(dbus->path,
+	if (ofono_dbus_clients_count(dbus->clients)) {
+		guint i;
+		DBusMessageIter it, a;
+		DBusMessage *signal = dbus_message_new_signal(dbus->path,
 					CELL_INFO_DBUS_INTERFACE, name);
 
-	dbus_message_iter_init_append(signal, &it);
-	dbus_message_iter_open_container(&it, DBUS_TYPE_ARRAY, "o", &array);
-	for (i = 0; i < list->len; i++) {
-		const char* path = list->pdata[i];
-		dbus_message_iter_append_basic(&array, DBUS_TYPE_OBJECT_PATH,
-								&path);
-	}
-	dbus_message_iter_close_container(&it, &array);
+		dbus_message_iter_init_append(signal, &it);
+		dbus_message_iter_open_container(&it, DBUS_TYPE_ARRAY, "o", &a);
+		for (i = 0; i < list->len; i++) {
+			const char* path = list->pdata[i];
 
-	sailfish_dbus_clients_send(dbus->clients, signal);
+			dbus_message_iter_append_basic(&a,
+					DBUS_TYPE_OBJECT_PATH, &path);
+		}
+		dbus_message_iter_close_container(&it, &a);
+		ofono_dbus_clients_signal(dbus->clients, signal);
+		dbus_message_ref(signal);
+	}
 }
 
 static int sailfish_cell_info_dbus_compare(const struct sailfish_cell *c1,
@@ -390,7 +392,7 @@ static void sailfish_cell_info_dbus_property_changed
 
 	for (i = 0; i < n && mask; i++) {
 		if (mask & prop[i].flag) {
-			sailfish_dbus_clients_signal_property_changed(
+			ofono_dbus_clients_signal_property_changed(
 				dbus->clients, entry->path,
 				CELL_DBUS_INTERFACE, prop[i].name,
 				DBUS_TYPE_INT32,
@@ -497,51 +499,56 @@ static void sailfish_cell_info_dbus_cells_changed_cb
 		((struct sailfish_cell_info_dbus *)arg, TRUE);
 }
 
+static DBusMessage *sailfish_cell_info_dbus_error_failed(DBusMessage *msg,
+						const char *explanation)
+{
+	return g_dbus_create_error(msg, OFONO_ERROR_INTERFACE ".Failed", "%s",
+								explanation);
+}
+
 static DBusMessage *sailfish_cell_info_dbus_get_cells(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct sailfish_cell_info_dbus *dbus = data;
-	struct sailfish_dbus_client *client;
+	const char *sender = dbus_message_get_sender(msg);
 
-	client = sailfish_dbus_clients_new_client(dbus->clients, msg);
-	if (client) {
+	if (ofono_dbus_clients_add(dbus->clients, sender)) {
 		DBusMessage *reply = dbus_message_new_method_return(msg);
-		DBusMessageIter it, array;
+		DBusMessageIter it, a;
 		GSList *l;
 
 		sailfish_cell_info_set_enabled(dbus->info, TRUE);
 		dbus_message_iter_init_append(reply, &it);
-		dbus_message_iter_open_container(&it, DBUS_TYPE_ARRAY,
-								"o", &array);
+		dbus_message_iter_open_container(&it, DBUS_TYPE_ARRAY, "o", &a);
 		for (l = dbus->entries; l; l = l->next) {
 			const struct sailfish_cell_entry *entry = l->data;
-			dbus_message_iter_append_basic(&array,
+
+			dbus_message_iter_append_basic(&a,
 					DBUS_TYPE_OBJECT_PATH, &entry->path);
 		}
-		dbus_message_iter_close_container(&it, &array);
+		dbus_message_iter_close_container(&it, &a);
 		return reply;
 	}
-	return __ofono_error_access_denied(msg);
+	return sailfish_cell_info_dbus_error_failed(msg, "Operation failed");
 }
 
 static DBusMessage *sailfish_cell_info_dbus_unsubscribe(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct sailfish_cell_info_dbus *dbus = data;
-	struct sailfish_dbus_client *client;
+	const char *sender = dbus_message_get_sender(msg);
 
-	DBG("");
-	client = sailfish_dbus_clients_lookup_client(dbus->clients, msg);
-	if (client) {
-		DBusMessage *signal = dbus_message_new_signal(
-					dbus_message_get_path(msg),
-					CELL_INFO_DBUS_INTERFACE,
-					CELL_INFO_DBUS_UNSUBSCRIBED_SIGNAL);
-		sailfish_dbus_clients_send_to(client, signal);
-		sailfish_dbus_clients_remove_client(client);
+	DBG("%s", sender);
+	if (ofono_dbus_clients_remove(dbus->clients, sender)) {
+		DBusMessage *signal = dbus_message_new_signal(dbus->path,
+			CELL_INFO_DBUS_INTERFACE,
+			CELL_INFO_DBUS_UNSUBSCRIBED_SIGNAL);
+
+		dbus_message_set_destination(msg, sender);
+		g_dbus_send_message(dbus->conn, signal);
 		return dbus_message_new_method_return(msg);
 	}
-	return __ofono_error_access_denied(msg);
+	return sailfish_cell_info_dbus_error_failed(msg, "Not subscribed");
 }
 
 static const GDBusMethodTable sailfish_cell_info_dbus_methods[] = {
@@ -558,13 +565,18 @@ static const GDBusSignalTable sailfish_cell_info_dbus_signals[] = {
 			GDBUS_ARGS({ "paths", "ao" })) },
 	{ GDBUS_SIGNAL(CELL_INFO_DBUS_CELLS_REMOVED_SIGNAL,
 			GDBUS_ARGS({ "paths", "ao" })) },
+	{ GDBUS_SIGNAL(CELL_INFO_DBUS_UNSUBSCRIBED_SIGNAL,
+			GDBUS_ARGS({})) },
 	{ }
 };
 
-static void sailfish_cell_info_dbus_disable_cb(void *user_data)
+static void sailfish_cell_info_dbus_disconnect_cb(const char *name, void *data)
 {
-	struct sailfish_cell_info_dbus *dbus = user_data;
-	sailfish_cell_info_set_enabled(dbus->info, FALSE);
+	struct sailfish_cell_info_dbus *dbus = data;
+
+	if (!ofono_dbus_clients_count(dbus->clients)) {
+		sailfish_cell_info_set_enabled(dbus->info, FALSE);
+	}
 }
 
 struct sailfish_cell_info_dbus *sailfish_cell_info_dbus_new
@@ -591,10 +603,8 @@ struct sailfish_cell_info_dbus *sailfish_cell_info_dbus_new
 			ofono_modem_add_interface(modem,
 						CELL_INFO_DBUS_INTERFACE);
 			sailfish_cell_info_dbus_update_entries(dbus, FALSE);
-			dbus->clients =
-				sailfish_dbus_clients_new(dbus->conn,
-					sailfish_cell_info_dbus_disable_cb,
-					dbus);
+			dbus->clients = ofono_dbus_clients_new(dbus->conn,
+				sailfish_cell_info_dbus_disconnect_cb, dbus);
 			return dbus;
 		} else {
 			ofono_error("CellInfo D-Bus register failed");
@@ -610,7 +620,7 @@ void sailfish_cell_info_dbus_free(struct sailfish_cell_info_dbus *dbus)
 		GSList *l;
 
 		DBG("%s", dbus->path);
-		sailfish_dbus_clients_free(dbus->clients);
+		ofono_dbus_clients_free(dbus->clients);
 		g_dbus_unregister_interface(dbus->conn, dbus->path,
 					CELL_INFO_DBUS_INTERFACE);