/soc/2015/igor.gajowiak/chatlog: 40d552c5ab9b: Add generic log d...

Changeset: 40d552c5ab9bed9ae6a3c38b41a66b832d79134f
Author:	 Igor Gajowiak <[email protected]>
Date:	 2015-07-16 07:37 +0200
Branch:	 default
URL: https://hg.pidgin.im/soc/2015/igor.gajowiak/chatlog/rev/40d552c5ab9b

Description:

Add generic log documentation comments and fix the generic log interface.

diffstat:

 libpurple/accounts.c              |   14 +-
 libpurple/conversation.c          |    3 +-
 libpurple/genericlog.c            |  328 ++++++++++++++++++++++++-------------
 libpurple/genericlog.h            |  218 +++++++++++++++++++++---
 libpurple/plugins/log/logsqlite.c |   74 ++-----
 5 files changed, 430 insertions(+), 207 deletions(-)

diffs (truncated from 957 to 300 lines):

diff --git a/libpurple/accounts.c b/libpurple/accounts.c
--- a/libpurple/accounts.c
+++ b/libpurple/accounts.c
@@ -930,17 +930,11 @@ show_unread_im_msgs(PurpleConnection *gc
 
 	g_return_if_fail(account != NULL);
 
-	GList *unread_msgs = NULL;
-	GError *error = purple_genericlog_get_unread_msgs(account, &unread_msgs);
-	if(error){
-		g_error_free(error);
+	GList *unseen_msgs = NULL;
+	if (!purple_genericlog_get_unseen_msgs(account, &unseen_msgs, NULL))
 		return;
-	}
 
-	g_return_if_fail(unread_msgs != NULL);
-
-	GList *it = NULL;
-	for(it = unread_msgs; it != NULL; it = it->next) {
+	for(GList *it = unseen_msgs; it != NULL; it = it->next) {
 		PurpleMessage *msg = it->data;
 
 		// Create or get a conversation
@@ -949,7 +943,7 @@ show_unread_im_msgs(PurpleConnection *gc
 
 		purple_conversation_write_message(PURPLE_CONVERSATION(conv), msg);
 	}
-	g_list_free_full(unread_msgs, (GDestroyNotify) g_object_unref);
+	g_list_free_full(unseen_msgs, (GDestroyNotify) g_object_unref);
 }
 
 void
diff --git a/libpurple/conversation.c b/libpurple/conversation.c
--- a/libpurple/conversation.c
+++ b/libpurple/conversation.c
@@ -633,7 +633,8 @@ void
 			ops->write_conv(conv, pmsg);
 	}
 
-	purple_genericlog_logim(account, pmsg);
+	// TODO: add error handling?
+	purple_genericlog_logim(account, pmsg, NULL);
 	add_message_to_history(conv, pmsg);
 
 	purple_signal_emit(purple_conversations_get_handle(),
diff --git a/libpurple/genericlog.c b/libpurple/genericlog.c
--- a/libpurple/genericlog.c
+++ b/libpurple/genericlog.c
@@ -48,53 +48,62 @@ static PurpleGenericLog *purple_genericl
 static GList *purple_genericlog_loaded_plugins = NULL;
 
 /**************************************************************************/
-/* API implementation                                                     */
+/* PurpleGenericLog intance methods                                       */
 /**************************************************************************/
 
-PurpleGenericLog*
-purple_genericlog_find_by_id(const gchar* id)
+const gchar *
+purple_genericlog_get_id(const PurpleGenericLog *log)
 {
-	GList *it;
-	for (it = purple_genericlogs; it != NULL; it = it->next) {
-		PurpleGenericLog *log = it->data;
-		const gchar *curr_id = purple_genericlog_get_id(log);
-
-		if (g_strcmp0(id, curr_id) == 0)
-			return log;
-	}
-	return NULL;
+	g_return_val_if_fail(log != NULL, NULL);
+	PurpleGenericLogPrivate *p_log = PURPLE_GENERICLOG_GET_PRIVATE(log);
+	return p_log->id;
 }
 
-PurpleGenericLog*
+const gchar *
+purple_genericlog_get_name(const PurpleGenericLog *log)
+{
+	g_return_val_if_fail(log != NULL, NULL);
+	PurpleGenericLogPrivate *log_priv = PURPLE_GENERICLOG_GET_PRIVATE(log);
+	return log_priv->name;
+}
+
+/**************************************************************************/
+/* Log subsystem API                                                      */
+/**************************************************************************/
+
+static gint
+id_compare(gconstpointer log, gconstpointer id)
+{
+	return g_strcmp0(purple_genericlog_get_id(log), id);
+}
+
+PurpleGenericLog *
+purple_genericlog_find_by_id(const gchar *id)
+{
+	g_return_val_if_fail(id != NULL, NULL);
+
+	GList *element = g_list_find_custom(purple_genericlogs,
+		id, id_compare);
+	return element ? element->data : NULL;
+}
+
+PurpleGenericLog *
 purple_genericlog_get_inuse()
 {
 	return purple_genericlog_inuse;
 }
 
-const gchar*
-purple_genericlog_get_name(const PurpleGenericLog* log)
+void
+purple_genericlog_register(PurpleGenericLog *log)
 {
-	PurpleGenericLogPrivate *log_priv = PURPLE_GENERICLOG_GET_PRIVATE(log);
-	return log_priv->name;
-}
+	g_return_if_fail(log != NULL);
 
-const gchar*
-purple_genericlog_get_id(const PurpleGenericLog* log)
-{
-	PurpleGenericLogPrivate *p_log = PURPLE_GENERICLOG_GET_PRIVATE(log);
-	return p_log->id;
-}
-
-void
-purple_genericlog_register(PurpleGenericLog* log)
-{
-	const gchar *id = NULL;
-	id = purple_genericlog_get_id(log);
-
+	const gchar *id = purple_genericlog_get_id(log);
 	g_return_if_fail(id != NULL);
 
 	if(purple_genericlog_find_by_id(id) != NULL) {
-		purple_debug_info("genericlog", "logger is already registered");
+		purple_debug_info("genericlog",
+			"Log with id='%s' is already registered", id);
 		return;
 	}
 
@@ -103,24 +112,23 @@ purple_genericlog_register(PurpleGeneric
 }
 
 void
-purple_genericlog_unregister(PurpleGenericLog* log)
+purple_genericlog_unregister(PurpleGenericLog *log)
 {
-	PurpleGenericLog *fallback;
-
 	g_return_if_fail(log != NULL);
 
 	GList* log_node = g_list_find(purple_genericlogs, log);
-
 	g_return_if_fail(log_node != NULL);
 
-	fallback = purple_genericlog_find_by_id(PURPLE_GENERICLOG_DEFAULT);
+	if (purple_genericlog_inuse == log) {
+		PurpleGenericLog *fallback = purple_genericlog_find_by_id(
+			PURPLE_GENERICLOG_DEFAULT);
 
-	if (purple_genericlog_inuse == log) {
-		if (purple_genericlog_inuse != fallback) {
-			purple_genericlog_set_active_log(PURPLE_GENERICLOG_DEFAULT);
-		} else {
-			purple_debug_error("genericlog", "unable to unregister the default log");
-			return;
+		if (purple_genericlog_inuse == fallback)
+			fallback = NULL;
+
+		if (!purple_genericlog_set_active_log(fallback, NULL)) {
+			purple_debug_error("genericlog", "Setting active log to %p failed",
+				fallback);
 		}
 	}
 
@@ -128,69 +136,165 @@ purple_genericlog_unregister(PurpleGener
 	purple_genericlogs = g_list_delete_link(purple_genericlogs, log_node);
 }
 
-GError*
-purple_genericlog_set_active_log(const gchar* id)
+gboolean
+purple_genericlog_set_active_log(PurpleGenericLog *log, GError **error)
 {
-	GError *error = NULL;
-	PurpleGenericLog *log = purple_genericlog_find_by_id(id);
+	if (log == purple_genericlog_inuse)
+		return TRUE;
 
-	if(!log) {
-		return g_error_new(purple_genericlog_error_domain(),
-			PURPLE_GENERICLOG_ERROR_NOLOG, "log does not exist");
+	if (log && !g_list_find(purple_genericlogs, log)) {
+		if (!error) return FALSE;
+
+		*error = g_error_new(purple_genericlog_error_domain(),
+			PURPLE_GENERICLOG_ERROR_NOLOG, "No log registered with id='%s'",
+			purple_genericlog_get_id(log));
+		return FALSE;
 	}
 
-	if(purple_genericlog_inuse) {
-		if(strcmp(id, purple_genericlog_get_id(purple_genericlog_inuse)) == 0) {
-			purple_debug_info("genericlog", "log is already active");
-			return NULL;
+	// First try to activate the new log
+	if (log) {
+		PurpleGenericLogClass *klass = PURPLE_GENERICLOG_GET_CLASS(log);
+
+		// Roll back if activation fails
+		if (!klass->activate()) {
+			if (!error) return FALSE;
+
+			*error = g_error_new(purple_genericlog_error_domain(),
+				PURPLE_GENERICLOG_ERROR_BACKENDFAIL, "Failed to activate %s",
+				purple_genericlog_get_id(log));
+			return FALSE;
 		}
-		else {
-			PurpleGenericLogClass *klass = PURPLE_GENERICLOG_GET_CLASS(purple_genericlog_inuse);
-			error = klass->deactivate();
-			if(error)
-				return error;
 
-			g_object_unref(purple_genericlog_inuse);
-			purple_genericlog_inuse = NULL;
-		}
+		g_object_ref(log);
 	}
 
-	g_assert(purple_genericlog_inuse == NULL);
+	// Now we can safely deactivate the previous log
+	if (purple_genericlog_inuse) {
+		PurpleGenericLogClass *klass = PURPLE_GENERICLOG_GET_CLASS(
+			purple_genericlog_inuse);
 
-	PurpleGenericLogClass *klass = PURPLE_GENERICLOG_GET_CLASS(log);
-	error = klass->activate();
-	// TODO: should we activate previous log if this fails?
-	if (error)
-		return error;
+		// This never fails
+		klass->deactivate();
+
+		g_object_unref(purple_genericlog_inuse);
+	}
 
 	purple_genericlog_inuse = log;
-	g_object_ref(purple_genericlog_inuse);
 
-	return NULL;
+	return TRUE;
 }
 
-GError*
-purple_genericlog_logim(PurpleAccount *account, PurpleMessage *msg)
+gboolean
+purple_genericlog_logim(PurpleAccount *account, PurpleMessage *msg,
+	GError **error)
 {
+	if (!purple_genericlog_inuse) {
+		if (!error) return FALSE;
+
+		*error = g_error_new(purple_genericlog_error_domain(),
+			PURPLE_GENERICLOG_ERROR_NOLOG, "No active log");
+		return FALSE;
+	}
+
 	PurpleGenericLogClass *klass =
 		PURPLE_GENERICLOG_GET_CLASS(purple_genericlog_inuse);
-	return klass->log_im(account, msg);
+
+	if (!klass->log_im) {
+		if (!error) return FALSE;
+
+		*error = g_error_new(purple_genericlog_error_domain(),
+			PURPLE_GENERICLOG_ERROR_OPERATIONNOTSUPPORTED,
+			"%s does not support IM logging",
+			purple_genericlog_get_id(purple_genericlog_inuse));
+		return FALSE;
+	}
+
+	if (!klass->log_im(account, msg)) {
+		if (!error) return FALSE;
+
+		*error = g_error_new(purple_genericlog_error_domain(),
+			PURPLE_GENERICLOG_ERROR_BACKENDFAIL,
+			"Failed to log IM in %s",
+			purple_genericlog_get_id(purple_genericlog_inuse));
+		return FALSE;
+	}
+
+	return TRUE;
 }
 
-GError*

_______________________________________________
Commits mailing list
[email protected]
https://pidgin.im/cgi-bin/mailman/listinfo/commits