From 0fbbb875dc27cc92ab57078de68ab98e9404266c Mon Sep 17 00:00:00 2001
From: Yuanle Song <sylecn@gmail.com>
Date: Mon, 8 Apr 2019 19:36:19 +0800
Subject: [PATCH] v0.6.0 support CommitCandidate

- CommitCandidate allow save phrase and user_freq in user db.
- bugfix: GetCandidates SQL query ORDER BY field fixed.
- bugfix: fix a memleak in on_handle_get_candidates()
- minor bugfix: fix variable name in meson.build
---
 ...yinService1.ZeroPinyinServiceInterface.xml |  15 ++
 main.c                                        |  23 +-
 meson.build                                   |  14 +-
 zero-pinyin-service-test.c                    |  19 +-
 zero-pinyin-service.c                         | 234 ++++++++++++++++--
 zero-pinyin-service.h                         |  26 +-
 6 files changed, 298 insertions(+), 33 deletions(-)

diff --git a/com.emacsos.zero.ZeroPinyinService1.ZeroPinyinServiceInterface.xml b/com.emacsos.zero.ZeroPinyinService1.ZeroPinyinServiceInterface.xml
index 24401bc..4be7072 100644
--- a/com.emacsos.zero.ZeroPinyinService1.ZeroPinyinServiceInterface.xml
+++ b/com.emacsos.zero.ZeroPinyinService1.ZeroPinyinServiceInterface.xml
@@ -22,6 +22,21 @@
       <arg type="u" name="fetch_size"/>
       <arg type="as" name="candidates" direction="out"/>
       <arg type="au" name="matched_preedit_str_lengths" direction="out"/>
+      <arg type="aa(ii)" name="candidates_pinyin_indices" direction="out"/>
+    </method>
+
+    <!-- CommitCandidate
+
+	 @candidate: the candidate user committed
+	 @candidate_pinyin_indices: the corresponding element in GetCandidates
+	                            candidates_pinyin_indices out param.
+
+	 This will allow zero-pinyin-service to add candidate to user db and
+	 revert DeleteCandidate effect.
+    -->
+    <method name="CommitCandidate">
+      <arg type="s" name="candidate"/>
+      <arg type="a(ii)" name="candidate_pinyin_indices"/>
     </method>
 
     <!-- DeleteCandidate
diff --git a/main.c b/main.c
index a1be78d..fa6edcd 100644
--- a/main.c
+++ b/main.c
@@ -33,14 +33,16 @@ on_handle_get_candidates (ZeroPinyinService *object,
 	GVariant *result = NULL;
 	GVariantBuilder *candidates_builder = NULL;
 	GVariantBuilder *matched_lengths_builder = NULL;
+	GVariantBuilder *candidates_pinyin_indices = NULL;
 
         /* test data */
 	/* get_candidates_test (preedit_str, fetch_size, candidates_builder, matched_lengths_builder); */
 	candidates_builder = g_variant_builder_new (G_VARIANT_TYPE ("as"));
 	matched_lengths_builder = g_variant_builder_new (G_VARIANT_TYPE ("au"));
-	get_candidates (appdata->db, preedit_str, fetch_size, candidates_builder, matched_lengths_builder);
+	candidates_pinyin_indices = g_variant_builder_new (G_VARIANT_TYPE ("aa(ii)"));
+	get_candidates (appdata->db, preedit_str, fetch_size, candidates_builder, matched_lengths_builder, candidates_pinyin_indices);
 
-	result = g_variant_new ("(asau)", candidates_builder, matched_lengths_builder);
+	result = g_variant_new ("(asauaa(ii))", candidates_builder, matched_lengths_builder, candidates_pinyin_indices);
 	g_assert_nonnull (result);
 
 	/* result is a GVarient tuple of two dbus arrays */
@@ -48,10 +50,23 @@ on_handle_get_candidates (ZeroPinyinService *object,
 
 	g_variant_builder_unref (candidates_builder);
 	g_variant_builder_unref (matched_lengths_builder);
+	g_variant_builder_unref (candidates_pinyin_indices);
 
 	return TRUE;
 }
 
+static gboolean
+on_handle_commit_candidate (ZeroPinyinService *object,
+			    GDBusMethodInvocation *invocation,
+			    const gchar *candidate,
+			    GVariant *candidate_pinyin_indices,
+			    AppData *appdata)
+{
+	commit_candidate (appdata->db, candidate, candidate_pinyin_indices);
+	g_dbus_method_invocation_return_value (invocation, NULL);
+	return TRUE;
+}
+
 static gboolean
 on_handle_delete_candidate (ZeroPinyinService *object,
 			    GDBusMethodInvocation *invocation,
@@ -118,6 +133,10 @@ on_bus_acquired (GDBusConnection *connection,
 			  "handle-get-candidates",
 			  G_CALLBACK (on_handle_get_candidates),
 			  appdata);
+	g_signal_connect (appdata->interface,
+			  "handle-commit-candidate",
+			  G_CALLBACK (on_handle_commit_candidate),
+			  appdata);
 	g_signal_connect (appdata->interface,
 			  "handle-delete-candidate",
 			  G_CALLBACK (on_handle_delete_candidate),
diff --git a/meson.build b/meson.build
index 8defd75..48087ec 100644
--- a/meson.build
+++ b/meson.build
@@ -1,6 +1,6 @@
 # -*- mode: conf -*-
 project('zero-pinyin-service', ['c', 'cpp'],
-  version: '0.5.1',
+  version: '0.6.0',
   license: 'GPL',
   default_options: [
     'warning_level=2',
@@ -44,17 +44,19 @@ shared_dep = [glib, gio, uuid, sqlite3]
 gen_inc = include_directories('.')
 
 gdbus_codegen = find_program('gdbus-codegen')
-zero_panel_generated = custom_target('zero-panel-generated',
+zero_pinyin_generated = custom_target('zero-panel-generated',
   input: 'com.emacsos.zero.ZeroPinyinService1.ZeroPinyinServiceInterface.xml',
   output: ['zero-pinyin-service-generated.h', 'zero-pinyin-service-generated.c'],
   command: [gdbus_codegen, '--generate-c-code', 'zero-pinyin-service-generated', '@INPUT@'])
 
-src = [
+lib = [
   '../PinyinParser.cc',
   '../sqlite3_util.c',
   'parse-pinyin.cpp',
-  'zero-pinyin-service.c',
-  zero_panel_generated,
+  'zero-pinyin-service.c']
+src = [
+  lib,
+  zero_pinyin_generated,
   'main.c']
 executable('zero-pinyin-service', src,
   include_directories: gen_inc,
@@ -67,5 +69,5 @@ test('parse-pinyin',
     dependencies: shared_dep))
 test('zero-pinyin-service-test',
   executable('zero-pinyin-service-test',
-    ['zero-pinyin-service-test.c'],
+    [lib, 'zero-pinyin-service-test.c'],
     dependencies: shared_dep))
diff --git a/zero-pinyin-service-test.c b/zero-pinyin-service-test.c
index dd29de4..8f43cef 100644
--- a/zero-pinyin-service-test.c
+++ b/zero-pinyin-service-test.c
@@ -4,7 +4,7 @@ static void
 test_GString ()
 {
 	GString* s = NULL;
-	s = g_string_new ("");
+	s = g_string_new (NULL);
 	g_string_append_printf (s, "s0=%d ", 1);
 
 	g_assert_cmpstr (s->str, ==, "s0=1 ");
@@ -12,11 +12,24 @@ test_GString ()
 	g_string_free (s, TRUE);
 }
 
+static void
+test_build_s_y_fields ()
+{
+	gchar* result = NULL;
+	result = build_s_y_fields (1);
+	g_assert_cmpstr (result, ==, ", s0, y0 ");
+	g_free (result);
+
+	result = build_s_y_fields (2);
+	g_assert_cmpstr (result, ==, ", s0, y0, s1, y1 ");
+	g_free (result);
+}
+
 int
 main (int argc, char *argv[])
 {
 	g_test_init (&argc, &argv, NULL);
-	g_test_add_func ("/zero/test_GString",
-			 test_GString);
+	g_test_add_func ("/zero/test_GString", test_GString);
+	g_test_add_func ("/zero/test_build_s_y_fields", test_build_s_y_fields);
 	return g_test_run ();
 }
diff --git a/zero-pinyin-service.c b/zero-pinyin-service.c
index 3796102..4ed3aab 100644
--- a/zero-pinyin-service.c
+++ b/zero-pinyin-service.c
@@ -1,5 +1,6 @@
 #include "zero-pinyin-service.h"
 #include "parse-pinyin.h"
+#include "../sqlite3_util.h"
 
 void
 get_candidates_test (const char* preedit_str,
@@ -47,7 +48,7 @@ build_where_clause (GList* pylist,
 	GList* iter = pylist;
 	gboolean first_condition_done = FALSE;
 	Pinyin* thispy = NULL;
-	s = g_string_new ("");
+	s = g_string_new (NULL);
 	for (guint i = 0; i < n; ++i) {
 		g_assert_nonnull (iter);
 		thispy = (Pinyin*) iter->data;
@@ -74,6 +75,26 @@ build_where_clause (GList* pylist,
 	return result;
 }
 
+/**
+ * return a string like ", s0, y0, s1, y1 "
+ *
+ * caller should g_free() result after use.
+ */
+char*
+build_s_y_fields (const guint n)
+{
+	GString *s = NULL;
+	g_assert_cmpint (n, >=, 1);
+	s = g_string_new (NULL);
+	for (guint i = 0; i < n; ++i) {
+		g_string_append_printf (s, ", s%u, y%u", i, i);
+	}
+	s = g_string_append (s, " ");
+	gchar *result = s->str;
+	g_string_free (s, FALSE);
+	return result;
+}
+
 /**
  * build a SQL to query candidates for first n pinyin in pylist.
  * n can be from 1 to len(pylist).
@@ -88,25 +109,35 @@ build_sql_for_n_pinyin (GList* pylist,
 	/* always keep one space after current term */
 	GString* sql = NULL;
 	gchar* where_clause = NULL;
-	sql = g_string_new ("SELECT MAX(user_freq), phrase, MAX(freq) FROM (");
+	sql = g_string_new ("SELECT MAX(user_freq) AS user_freq, "
+			    "phrase, MAX(freq) AS freq");
+	gchar* s_y_fields = build_s_y_fields (n);
+	g_string_append_printf (sql, s_y_fields);
+	g_string_append_printf (sql, "FROM (");
+	g_string_append_printf (
+		sql, "SELECT 0 AS user_freq, phrase, freq");
+	g_string_append_printf (sql, s_y_fields);
 	g_string_append_printf (
-		sql, "SELECT 0 AS user_freq, phrase, freq FROM "
-		"maindb.py_phrase_%u WHERE ", n - 1);
+		sql, "FROM maindb.py_phrase_%u WHERE ", n - 1);
 	where_clause = build_where_clause (pylist, n);
 	g_assert_nonnull (where_clause);
-	g_message ("where_clause=%s", where_clause);
+	g_debug ("where_clause=%s", where_clause);
 	sql = g_string_append (sql, where_clause);
 	sql = g_string_append (sql, "UNION ");
 	g_string_append_printf (
-		sql, "SELECT user_freq, phrase, freq FROM "
-		"userdb.py_phrase_%u WHERE ", n - 1);
+		sql, "SELECT user_freq, phrase, freq");
+	g_string_append_printf (sql, s_y_fields);
+	g_string_append_printf (
+		sql, "FROM userdb.py_phrase_%u WHERE ", n - 1);
 	sql = g_string_append (sql, where_clause);
 	sql = g_string_append (
 		sql, ") "
 		"WHERE phrase NOT IN (SELECT phrase FROM userdb.not_phrase) "
-		"GROUP BY phrase ORDER BY user_freq DESC, freq DESC ");
+		"GROUP BY phrase "
+		"ORDER BY user_freq DESC, freq DESC ");
 	g_string_append_printf (sql, "LIMIT %u;", limit);
 	char* result = sql->str;
+	g_free (s_y_fields);
 	g_free (where_clause);
 	g_string_free (sql, FALSE);
 	return result;
@@ -177,7 +208,7 @@ get_candidates_for_n_pinyin (sqlite3* db,
 	/* build SQL and run SQL query */
 	char* sql = NULL;
 	sql = build_sql_for_n_pinyin (pylist, group_size, MAX (limit, DEFAULT_LIMIT));
-	g_message ("build_sql_for_n_pinyin result SQL:\n\n%s\n", sql);
+	g_debug ("build_sql_for_n_pinyin result SQL:\n\n%s\n", sql);
 
 	guint matched_py_length = get_matched_py_length (preedit_str, pylist, group_size);
 
@@ -197,12 +228,20 @@ get_candidates_for_n_pinyin (sqlite3* db,
 		if (r == SQLITE_DONE) {
 			break;
 		} else if (r == SQLITE_ROW) {
-			c = g_new (Candidate, 1);
-			/* sql SELECT should select these 3 columns in order */
+			c = g_new0 (Candidate, 1);
+			/* sql SELECT should select these columns in order */
 			c->user_freq = sqlite3_column_int (stmt, 0);
 			c->str = g_strdup ((const char*) sqlite3_column_text (stmt, 1));
 			c->freq = sqlite3_column_int (stmt, 2);
 			c->matched_py_length = matched_py_length;
+			c->char_len = group_size;
+			c->py_indices = g_malloc0 (sizeof (Pinyin*) * group_size);
+			for (guint i = 0; i < group_size; ++i) {
+				c->py_indices[i] = g_new0 (Pinyin, 1);
+				c->py_indices[i]->shengmu_i = sqlite3_column_int (stmt, 3 + i * 2);
+				c->py_indices[i]->yunmu_i = sqlite3_column_int (stmt, 4 + i * 2);
+				/* we don't care about ->length field */
+			}
 
 			if (g_utf8_validate (c->str, -1, NULL)) {
 				result = g_list_prepend (result, c);
@@ -229,12 +268,41 @@ get_candidates_for_n_pinyin (sqlite3* db,
 	return candidates_count;
 }
 
+static void
+add_candidate_to_builders (Candidate *c,
+			   GVariantBuilder *candidates_builder,
+			   GVariantBuilder *matched_lengths_builder,
+			   GVariantBuilder *candidates_pinyin_indices)
+{
+	g_variant_builder_add (candidates_builder, "s", c->str);
+	g_variant_builder_add (matched_lengths_builder, "u", c->matched_py_length);
+	GVariantBuilder *py_indices_builder = NULL;
+	py_indices_builder = g_variant_builder_new (G_VARIANT_TYPE ("a(ii)"));
+	for (guint i = 0; i < c->char_len; ++i) {
+		g_variant_builder_add (
+			py_indices_builder, "(ii)",
+			c->py_indices[i]->shengmu_i,
+			c->py_indices[i]->yunmu_i);
+		g_debug ("adding (ii) %d %d",
+			 c->py_indices[i]->shengmu_i,
+			 c->py_indices[i]->yunmu_i);
+		g_free (c->py_indices[i]);
+	}
+	g_debug ("adding a(ii) to aa(ii)");
+	g_variant_builder_add (candidates_pinyin_indices, "a(ii)",
+			       py_indices_builder);
+	g_variant_builder_unref (py_indices_builder);
+	g_free (c->str);
+	g_free (c->py_indices);
+}
+
 void
 get_candidates (sqlite3* db,
 		const char* preedit_str,
 		const guint fetch_size,
 		GVariantBuilder *candidates_builder,
-		GVariantBuilder *matched_lengths_builder)
+		GVariantBuilder *matched_lengths_builder,
+		GVariantBuilder *candidates_pinyin_indices)
 {
 	if (! db) {
 		g_warning ("No db connection, can't get candidates.");
@@ -258,11 +326,10 @@ get_candidates (sqlite3* db,
 			Candidate* c = NULL;
 			while (iter != NULL) {
 				c = (Candidate*) iter->data;
-				g_variant_builder_add (candidates_builder, "s",
-						       c->str);
-				g_free (c->str);
-				g_variant_builder_add (matched_lengths_builder, "u",
-						       c->matched_py_length);
+				add_candidate_to_builders (
+					c, candidates_builder,
+					matched_lengths_builder,
+					candidates_pinyin_indices);
 				iter = iter->next;
 			}
 			g_list_free_full (candidates, g_free);
@@ -274,3 +341,136 @@ get_candidates (sqlite3* db,
 	g_message ("returning %u candidates", fetched_size);
 	g_list_free_full (pylist, g_free);
 }
+
+/**
+ * sub function for commit_candidate()
+ */
+static void
+_update_userdb_py_phrase (sqlite3 *db,
+			  const gchar *candidate,
+			  GVariant *candidate_pinyin_indices,
+			  guint len) /* utf8 length of candidate char */
+{
+	GString *sql = NULL;
+	GVariantIter iter = {0};
+	GVariant *child = NULL;
+	gint x = 0;
+	gint y = 0;
+	guint count = 0;
+	char *s = NULL;
+	gboolean rb = FALSE;
+
+	g_assert_nonnull (db);
+	g_assert_nonnull (candidate);
+	g_assert_nonnull (candidate_pinyin_indices);
+
+	/* insert candidate maybe */
+
+	sql = g_string_new (NULL);
+	g_string_append_printf (sql, "INSERT OR IGNORE INTO userdb.py_phrase_%u (user_freq, phrase, freq", len - 1);
+	gchar* s_y_fields = build_s_y_fields (len);
+	sql = g_string_append (sql, s_y_fields);
+	g_free (s_y_fields);
+	s = sqlite3_mprintf (") VALUES (0, %Q, 0", candidate);
+	sql = g_string_append (sql, s);
+	sqlite3_free (s);
+	/* iter over GVariant "a(ii)" */
+	g_variant_iter_init (&iter, candidate_pinyin_indices);
+	count = 0;
+	while ((child = g_variant_iter_next_value (&iter))) {
+		g_variant_get (child, "(ii)", &x, &y);
+		g_string_append_printf (sql, ", %d, %d", x, y);
+		count++;
+	}
+	if (count != len) {
+		g_warning ("candidate length=%u, a(ii) length=%u, mismatch!",
+			   len, count);
+		g_string_free (sql, TRUE);
+		g_assert_not_reached ();
+		return;
+	}
+	g_string_append_printf (sql, ");");
+	rb = sqlite3_exec_simple (db, sql->str);
+	if (! rb) {
+		g_warning ("INSERT candidate to userdb failed");
+	} else {
+		if (sqlite3_changes (db) == 1) {
+			g_message ("candidate %s inserted to userdb", candidate);
+		}
+	}
+	g_string_free (sql, TRUE);
+
+	/* increment user_freq field for candidate */
+
+	sql = g_string_new (NULL);
+	g_string_append_printf (sql, "UPDATE userdb.py_phrase_%u "
+				"SET user_freq = user_freq + 1 ", len - 1);
+	s = sqlite3_mprintf ("WHERE phrase = %Q ", candidate);
+	sql = g_string_append (sql, s);
+	sqlite3_free (s);
+	g_variant_iter_init (&iter, candidate_pinyin_indices);
+	count = 0;
+	while ((child = g_variant_iter_next_value (&iter))) {
+		g_variant_get (child, "(ii)", &x, &y);
+		g_string_append_printf (sql, "AND s%d=%d AND y%d=%d ",
+					count, x, count, y);
+		count++;
+	}
+	sql = g_string_append (sql, ";");
+	rb = sqlite3_exec_simple (db, sql->str);
+	if (! rb) {
+		g_warning ("UPDATE candidate user_freq failed");
+	} else {
+		if (sqlite3_changes (db) == 1) {
+			g_message ("candidate %s user_freq incremented", candidate);
+		} else {
+			g_warning ("UPDATE candidate user_freq failed, no match");
+		}
+	}
+	g_string_free (sql, TRUE);
+}
+
+static void
+_update_userdb_not_phrase (sqlite3 *db,
+			   const gchar *candidate)
+{
+	g_assert_nonnull (db);
+	g_assert_nonnull (candidate);
+	gboolean rb = FALSE;
+	char *sql = sqlite3_mprintf ("DELETE FROM userdb.not_phrase WHERE phrase = %Q;", candidate);
+	rb = sqlite3_exec_simple (db, sql);
+	if (! rb) {
+		g_warning ("DELETE candidate from not_phrase failed");
+	} else {
+		if (sqlite3_changes (db) == 1) {
+			g_message ("candidate %s removed from not_phrase", candidate);
+		}
+	}
+	sqlite3_free (sql);
+}
+
+void
+commit_candidate (sqlite3 *db,
+		  const gchar *candidate,
+		  GVariant *candidate_pinyin_indices)
+{
+	if (! db) {
+		g_warning ("No db connection, can't commit candidates.");
+		return;
+	}
+	if (! candidate) {
+		g_warning ("candidate should not be NULL. won't commit candidate.");
+		return;
+	}
+	if (! candidate_pinyin_indices) {
+		g_warning ("candidate_pinyin_indices should not be NULL. won't commit candidate.");
+		return;
+	}
+	guint len = g_utf8_strlen (candidate, -1);
+	if (len <= 1) {
+		g_message ("commit single character %s is a no-op", candidate);
+		return;
+	}
+	_update_userdb_py_phrase (db, candidate, candidate_pinyin_indices, len);
+	_update_userdb_not_phrase (db, candidate);
+}
diff --git a/zero-pinyin-service.h b/zero-pinyin-service.h
index b8e88c1..13d0d19 100644
--- a/zero-pinyin-service.h
+++ b/zero-pinyin-service.h
@@ -18,16 +18,18 @@ typedef struct {
 } Pinyin;
 
 typedef struct {
-	gchar* str;		/* the candidate string */
+	gchar *str;		/* the candidate string */
 	guint freq;		/* word frequency [0, 65535] */
 	guint user_freq;	/* user frequency [0, 65535] */
 	guint matched_py_length; /* matched preedit_str length */
+	guint char_len;		 /* candidate Chinese character length */
+	Pinyin **py_indices;	 /* Pinyin for each character in this candidate */
 } Candidate;
 
 /**
  * an implementation of get_candidates() with simple test data.
  */
-void get_candidates_test (const char* preedit_str,
+void get_candidates_test (const char *preedit_str,
 			  const guint fetch_size,
 			  GVariantBuilder *candidates_builder,
 			  GVariantBuilder *matched_lengths_builder);
@@ -40,11 +42,25 @@ void get_candidates_test (const char* preedit_str,
  * @candidates_builder candidates will be added to this builder
  * @matched_lengths_builder matched preedit_str length will be added to this builder
  */
-void get_candidates (sqlite3* db,
-		     const char* preedit_str,
+void get_candidates (sqlite3 *db,
+		     const char *preedit_str,
 		     const guint fetch_size,
 		     GVariantBuilder *candidates_builder,
-		     GVariantBuilder *matched_lengths_builder);
+		     GVariantBuilder *matched_lengths_builder,
+		     GVariantBuilder *candidates_pinyin_indices);
+
+/**
+ * commit candidate.
+ *
+ * This will save candidate in user db so it's available in the future.
+ * It also update user_freq for given candidate.
+ */
+void commit_candidate (sqlite3 *db,
+		       const gchar *candidate,
+		       GVariant *candidate_pinyin_indices);
+
+/* for test only */
+char* build_s_y_fields (const guint n);
 
 G_END_DECLS
 
-- 
GitLab