* src/odbc/ejabberd_odbc.erl: get rid of SERIALIZABLE isolation level on MySQL connections.

* src/odbc/odbc_queries.erl: replaces all delete->insert chains with update->insert.
* src/mod_privacy_odbc.erl: moved sql queries to odbc_queries.erl.
* src/mod_roster_odbc.erl: changed interface for odbc_queries.erl.

SVN Revision: 1755

ChangeLog

@@ -1,3 +1,12 @@
+2008-12-26  Evgeniy Khramtsov <ekhramtsov@process-one.net>
+
+	* src/odbc/ejabberd_odbc.erl: get rid of SERIALIZABLE isolation
+	level on MySQL connections.
+	* src/odbc/odbc_queries.erl: replaces all delete->insert chains
+	with update->insert.
+	* src/mod_privacy_odbc.erl: moved sql queries to odbc_queries.erl.
+	* src/mod_roster_odbc.erl: changed interface for odbc_queries.erl.
+
 2008-12-24  Badlop  <badlop@process-one.net>
 
 	* src/aclocal.m4: Fixes in configure script: fix

src/mod_privacy_odbc.erl

@@ -768,132 +768,66 @@ item_to_raw(#listitem{type = Type,
     SMatchMessage = if MatchMessage -> "1"; true -> "0" end,
     SMatchPresenceIn = if MatchPresenceIn -> "1"; true -> "0" end,
     SMatchPresenceOut = if MatchPresenceOut -> "1"; true -> "0" end,
-    ["'", SType, "', "
+    [SType, SValue, SAction, SOrder, SMatchAll, SMatchIQ,
-     "'", SValue, "', "
+     SMatchMessage, SMatchPresenceIn, SMatchPresenceOut].
-     "'", SAction, "', "
-     "'", SOrder, "', "
-     "'", SMatchAll, "', "
-     "'", SMatchIQ, "', "
-     "'", SMatchMessage, "', "
-     "'", SMatchPresenceIn, "', "
-     "'", SMatchPresenceOut, "'"].
-
 
 sql_get_default_privacy_list(LUser, LServer) ->
     Username = ejabberd_odbc:escape(LUser),
-    ejabberd_odbc:sql_query(
+    odbc_queries:get_default_privacy_list(LServer, Username).
-      LServer,
-      ["select name from privacy_default_list "
-       "where username='", Username, "';"]).
 
 sql_get_default_privacy_list_t(LUser) ->
     Username = ejabberd_odbc:escape(LUser),
-    ejabberd_odbc:sql_query_t(
+    odbc_queries:get_default_privacy_list_t(Username).
-      ["select name from privacy_default_list "
-       "where username='", Username, "';"]).
 
 sql_get_privacy_list_names(LUser, LServer) ->
     Username = ejabberd_odbc:escape(LUser),
-    ejabberd_odbc:sql_query(
+    odbc_queries:get_privacy_list_names(LServer, Username).
-      LServer,
-      ["select name from privacy_list "
-       "where username='", Username, "';"]).
 
 sql_get_privacy_list_names_t(LUser) ->
     Username = ejabberd_odbc:escape(LUser),
-    ejabberd_odbc:sql_query_t(
+    odbc_queries:get_privacy_list_names_t(Username).
-      ["select name from privacy_list "
-       "where username='", Username, "';"]).
 
 sql_get_privacy_list_id(LUser, LServer, Name) ->
     Username = ejabberd_odbc:escape(LUser),
     SName = ejabberd_odbc:escape(Name),
-    ejabberd_odbc:sql_query(
+    odbc_queries:get_privacy_list_id(LServer, Username, SName).
-      LServer,
-      ["select id from privacy_list "
-       "where username='", Username, "' and name='", SName, "';"]).
 
 sql_get_privacy_list_id_t(LUser, Name) ->
     Username = ejabberd_odbc:escape(LUser),
     SName = ejabberd_odbc:escape(Name),
-    ejabberd_odbc:sql_query_t(
+    odbc_queries:get_privacy_list_id_t(Username, SName).
-      ["select id from privacy_list "
-       "where username='", Username, "' and name='", SName, "';"]).
 
 sql_get_privacy_list_data(LUser, LServer, Name) ->
     Username = ejabberd_odbc:escape(LUser),
     SName = ejabberd_odbc:escape(Name),
-    ejabberd_odbc:sql_query(
+    odbc_queries:get_privacy_list_data(LServer, Username, SName).
-      LServer,
-      ["select t, value, action, ord, match_all, match_iq, "
-       "match_message, match_presence_in, match_presence_out "
-       "from privacy_list_data "
-       "where id = (select id from privacy_list where "
-       "            username='", Username, "' and name='", SName, "') "
-       "order by ord;"]).
 
 sql_get_privacy_list_data_by_id(ID, LServer) ->
-    ejabberd_odbc:sql_query(
+    odbc_queries:get_privacy_list_data_by_id(LServer, ID).
-      LServer,
-      ["select t, value, action, ord, match_all, match_iq, "
-       "match_message, match_presence_in, match_presence_out "
-       "from privacy_list_data "
-       "where id='", ID, "' order by ord;"]).
 
 sql_set_default_privacy_list(LUser, Name) ->
     Username = ejabberd_odbc:escape(LUser),
     SName = ejabberd_odbc:escape(Name),
-    ejabberd_odbc:sql_query_t(
+    odbc_queries:set_default_privacy_list(Username, SName).
-      ["delete from privacy_default_list "
-       "      where username='", Username, "';"]),
-    ejabberd_odbc:sql_query_t(
-      ["insert into privacy_default_list(username, name) "
-       "values ('", Username, "', '", SName, "');"]).
 
 sql_unset_default_privacy_list(LUser, LServer) ->
     Username = ejabberd_odbc:escape(LUser),
-    ejabberd_odbc:sql_query(
+    odbc_queries:unset_default_privacy_list(LServer, Username).
-      LServer,
-      ["delete from privacy_default_list "
-       "      where username='", Username, "';"]).
 
 sql_remove_privacy_list(LUser, Name) ->
     Username = ejabberd_odbc:escape(LUser),
     SName = ejabberd_odbc:escape(Name),
-    ejabberd_odbc:sql_query_t(
+    odbc_queries:remove_privacy_list(Username, SName).
-      ["delete from privacy_list "
-       "where username='", Username, "' and name='", SName, "';"]).
 
 sql_add_privacy_list(LUser, Name) ->
     Username = ejabberd_odbc:escape(LUser),
     SName = ejabberd_odbc:escape(Name),
-    ejabberd_odbc:sql_query_t(
+    odbc_queries:add_privacy_list(Username, SName).
-      ["insert into privacy_list(username, name) "
-       "values ('", Username, "', '", SName, "');"]).
 
 sql_set_privacy_list(ID, RItems) ->
-    ejabberd_odbc:sql_query_t(
+    odbc_queries:set_privacy_list(ID, RItems).
-      ["delete from privacy_list_data "
-       "where id='", ID, "';"]),
-    lists:foreach(fun(Items) ->
-			  ejabberd_odbc:sql_query_t(
-			    ["insert into privacy_list_data("
-			     "id, t, value, action, ord, match_all, match_iq, "
-			     "match_message, match_presence_in, "
-			     "match_presence_out "
-			     ") "
-			     "values ('", ID, "', ", Items, ");"])
-		  end, RItems).
 
 sql_del_privacy_lists(LUser, LServer) ->
     Username = ejabberd_odbc:escape(LUser),
     Server = ejabberd_odbc:escape(LServer),
-    ejabberd_odbc:sql_query(
+    odbc_queries:del_privacy_lists(LServer, Server, Username).
-      LServer,
-      ["delete from privacy_list where username='", Username, "';"]),
-    ejabberd_odbc:sql_query(
-      LServer,
-      ["delete from privacy_list_data where value='", Username++"@"++Server, "';"]),
-    ejabberd_odbc:sql_query(
-      LServer,
-      ["delete from privacy_default_list where username='", Username, "';"]).

src/mod_roster_odbc.erl

@@ -858,13 +858,7 @@ record_to_string(#roster{us = {User, _Server},
 	       none	   -> "N"
 	   end,
     SAskMessage = ejabberd_odbc:escape(AskMessage),
-    ["'", Username, "',"
+    [Username, SJID, Nick, SSubscription, SAsk, SAskMessage, "N", "", "item"].
-     "'", SJID, "',"
-     "'", Nick, "',"
-     "'", SSubscription, "',"
-     "'", SAsk, "',"
-     "'", SAskMessage, "',"
-     "'N', '', 'item'"].
 
 groups_to_string(#roster{us = {User, _Server},
 			 jid = JID,
@@ -874,12 +868,11 @@ groups_to_string(#roster{us = {User, _Server},
 
     %% Empty groups do not need to be converted to string to be inserted in
     %% the database
-    lists:foldl(fun([], Acc) -> Acc;
+    lists:foldl(
+      fun([], Acc) -> Acc;
 	 (Group, Acc) ->
-			String = ["'", Username, "',"
+ 	      G = ejabberd_odbc:escape(Group),
-				  "'", SJID, "',"
+	      [[Username, SJID, G]|Acc] end, [], Groups).
-				  "'", ejabberd_odbc:escape(Group), "'"],
-			[String|Acc] end, [], Groups).
 
 webadmin_page(_, Host,
 	      #request{us = _US,

src/odbc/ejabberd_odbc.erl

@@ -93,12 +93,12 @@ sql_query_t(Query) ->
 	{error, "No SQL-driver information available."} ->
 	    % workaround for odbc bug
 	    {updated, 0};
-	{error, _} ->
+	{error, _} = Err ->
-	    throw(aborted);
+	    exit(Err);
 	Rs when is_list(Rs) ->
-	    case lists:keymember(error, 1, Rs) of
+	    case lists:keysearch(error, 1, Rs) of
-		true ->
+		{value, Err} ->
-		    throw(aborted);
+		    exit(Err);
 		_ ->
 		    QRes
 	    end;
@@ -330,11 +330,6 @@ mysql_connect(Server, Port, DB, Username, Password, StartInterval) ->
 	{ok, Ref} ->
 	    erlang:monitor(process, Ref),
             mysql_conn:fetch(Ref, ["set names 'utf8';"], self()),
-	    % needed to ensure the order of queries, specifically at
-	    % roster subscription time (this can also be set-up in the
-	    % MySQL configuration, but not at the database level):
-            mysql_conn:fetch(Ref, ["SET SESSION TRANSACTION ISOLATION LEVEL "
-				   "SERIALIZABLE;"], self()),
 	    {ok, #state{db_ref = Ref, db_type = mysql}};
 	{error, Reason} ->
 	    ?ERROR_MSG("MySQL connection failed: ~p~nWaiting ~p seconds before retrying...~n",

src/odbc/odbc_queries.erl

@@ -61,6 +61,20 @@
 	 set_private_data_sql/3,
 	 get_private_data/3,
 	 del_user_private_storage/2,
+	 get_default_privacy_list/2,
+	 get_default_privacy_list_t/1,
+	 get_privacy_list_names/2,
+	 get_privacy_list_names_t/1,
+	 get_privacy_list_id/3,
+	 get_privacy_list_id_t/2,
+	 get_privacy_list_data/3,
+	 get_privacy_list_data_by_id/2,
+	 set_default_privacy_list/2,
+	 unset_default_privacy_list/2,
+	 remove_privacy_list/2,
+	 add_privacy_list/2,
+	 set_privacy_list/2,
+	 del_privacy_lists/3,
 	 set_vcard/26,
 	 get_vcard/2,
 	 escape/1,
@@ -81,9 +95,28 @@
 get_db_type() ->
     generic.
 
+%% Safe atomic update.
+update_t(Table, Fields, Vals, Where) ->
+    UPairs = lists:zipwith(fun(A, B) -> A ++ "='" ++ B ++ "'" end,
+			   Fields, Vals),
+    case ejabberd_odbc:sql_query_t(
+	   ["update ", Table, " set ",
+	    string:join(UPairs, ", "),
+	    " where ", Where, ";"]) of
+	{updated, 1} ->
+	    ok;
+	_ ->
+	    %% The 'catch' herein is used because mysql returns
+	    %% affected rows (not matched as in postgresql).
+	    %% FIXME: need to find more suitable solution.
+	    catch ejabberd_odbc:sql_query_t(
+		    ["insert into ", Table, "(", string:join(Fields, ", "),
+		     ") values ('", string:join(Vals, "', '"), "');"])
+    end.
+
 %% F can be either a fun or a list of queries
-%% TODO: We should probably move the list of queries transaction wrapper from the ejabberd_odbc module
+%% TODO: We should probably move the list of queries transaction
-%%       to this one (odbc_queries)
+%% wrapper from the ejabberd_odbc module to this one (odbc_queries)
 sql_transaction(LServer, F) ->
     ejabberd_odbc:sql_transaction(LServer, F).
 
@@ -97,9 +130,11 @@ set_last_t(LServer, Username, Seconds, State) ->
     %% MREMOND: I think this should be turn into a non transactional behaviour
     ejabberd_odbc:sql_transaction(
       LServer,
-      [["delete from last where username='", Username, "';"],
+      fun() ->
-       ["insert into last(username, seconds, state) "
+	      update_t("last", ["username", "seconds", "state"],
-	"values ('", Username, "', '", Seconds, "', '", State, "');"]]).
+		       [Username, Seconds, State],
+		       ["username='", Username, "'"])
+      end).
 
 del_last(LServer, Username) ->
     ejabberd_odbc:sql_query(
@@ -115,9 +150,11 @@ get_password(LServer, Username) ->
 set_password_t(LServer, Username, Pass) ->
     ejabberd_odbc:sql_transaction(
       LServer,
-      [["delete from users where username='", Username ,"';"],
+      fun() ->
-       ["insert into users(username, password) "
+	      update_t("users", ["username", "password"],
-	"values ('", Username, "', '", Pass, "');"]]).
+		       [Username, Pass],
+		       ["username='", Username ,"'"])
+      end).
 
 add_user(LServer, Username, Pass) ->
     ejabberd_odbc:sql_query(
@@ -296,16 +333,11 @@ del_roster_sql(Username, SJID) ->
       "        and jid='", SJID, "';"]].
 
 update_roster(_LServer, Username, SJID, ItemVals, ItemGroups) ->
-    ejabberd_odbc:sql_query_t(
+    update_t("rosterusers",
-      ["delete from rosterusers "
+	     ["username", "jid", "nick", "subscription", "ask",
-       "      where username='", Username, "' "
+	      "askmessage", "server", "subscribe", "type"],
-       "        and jid='", SJID, "';"]),
+	     ItemVals,
-    ejabberd_odbc:sql_query_t(
+	     ["username='", Username, "' and jid='", SJID, "'"]),
-      ["insert into rosterusers("
-       "              username, jid, nick, "
-       "              subscription, ask, askmessage, "
-       "              server, subscribe, type) "
-       " values (", ItemVals, ");"]),
     ejabberd_odbc:sql_query_t(
       ["delete from rostergroups "
        "      where username='", Username, "' "
@@ -314,7 +346,7 @@ update_roster(_LServer, Username, SJID, ItemVals, ItemGroups) ->
 			  ejabberd_odbc:sql_query_t(
 			    ["insert into rostergroups("
 			     "              username, jid, grp) "
-			     " values (", ItemGroup, ");"])
+			     " values ('", string:join(ItemGroup, "', '"), "');"])
 		  end,
 		  ItemGroups).
 
@@ -326,26 +358,21 @@ update_roster_sql(Username, SJID, ItemVals, ItemGroups) ->
       "              username, jid, nick, "
       "              subscription, ask, askmessage, "
       "              server, subscribe, type) "
-      " values (", ItemVals, ");"],
+      " values ('", string:join(ItemVals, "', '"), "');"],
      ["delete from rostergroups "
       "      where username='", Username, "' "
       "        and jid='", SJID, "';"]] ++
      [["insert into rostergroups("
        "              username, jid, grp) "
-       " values (", ItemGroup, ");"] ||
+       " values ('", string:join(ItemGroup, "', '"), "');"] ||
 	 ItemGroup <- ItemGroups].
 
 roster_subscribe(_LServer, Username, SJID, ItemVals) ->
-    ejabberd_odbc:sql_query_t(
+    update_t("rosterusers",
-      ["delete from rosterusers "
+	     ["username", "jid", "nick", "subscription", "ask",
-       "      where username='", Username, "' "
+	      "askmessage", "server", "subscribe", "type"],
-       "        and jid='", SJID, "';"]),
+	     ItemVals,
-    ejabberd_odbc:sql_query_t(
+	     ["username='", Username, "' and jid='", SJID, "'"]).
-      ["insert into rosterusers("
-       "              username, jid, nick, "
-       "              subscription, ask, askmessage, "
-       "              server, subscribe, type) "
-       " values (", ItemVals, ");"]).
 
 get_subscription(LServer, Username, SJID) ->
     ejabberd_odbc:sql_query(
@@ -355,10 +382,10 @@ get_subscription(LServer, Username, SJID) ->
        "and jid='", SJID, "'"]).
 
 set_private_data(_LServer, Username, LXMLNS, SData) ->
-    lists:foreach(fun(Query) ->
+    update_t("private_storage",
-            ejabberd_odbc:sql_query_t(Query)
+	     ["username", "namespace", "data"],
-        end,
+	     [Username, LXMLNS, SData], 
-        set_private_data_sql(Username, LXMLNS, SData)).
+	     ["username='", Username, "' and namespace='", LXMLNS, "'"]).
 
 set_private_data_sql(Username, LXMLNS, SData) ->
     [["delete from private_storage "
@@ -380,35 +407,29 @@ del_user_private_storage(LServer, Username) ->
       LServer,
       ["delete from private_storage where username='", Username, "';"]).
 
-
 set_vcard(LServer, LUsername, SBDay, SCTRY, SEMail, SFN, SFamily, SGiven,
 	  SLBDay, SLCTRY, SLEMail, SLFN, SLFamily, SLGiven, SLLocality,
 	  SLMiddle, SLNickname, SLOrgName, SLOrgUnit, SLocality, SMiddle,
 	  SNickname, SOrgName, SOrgUnit, SVCARD, Username) ->
     ejabberd_odbc:sql_transaction(
       LServer,
-      [["delete from vcard where username='", LUsername, "';"],
+      fun() ->
-       ["insert into vcard(username, vcard) "
+	      update_t("vcard", ["username", "vcard"],
-	"values ('", LUsername, "', '", SVCARD, "');"],
+		       [LUsername, SVCARD],
-       ["delete from vcard_search where lusername='", LUsername, "';"],
+		       ["username='", LUsername, "'"]),
-       ["insert into vcard_search("
+	      update_t("vcard_search",
-	"        username, lusername, fn, lfn, family, lfamily,"
+		       ["username", "lusername", "fn", "lfn", "family",
-	"        given, lgiven, middle, lmiddle, nickname, lnickname,"
+			"lfamily", "given", "lgiven", "middle", "lmiddle",
-	"        bday, lbday, ctry, lctry, locality, llocality,"
+			"nickname", "lnickname", "bday", "lbday", "ctry",
-	"        email, lemail, orgname, lorgname, orgunit, lorgunit)"
+			"lctry", "locality", "llocality", "email", "lemail",
-	"values (",
+			"orgname", "lorgname", "orgunit", "lorgunit"],
-	"        '", Username,  "', '", LUsername,  "',"
+		       [Username, LUsername, SFN, SLFN, SFamily, SLFamily,
-	"        '", SFN,       "', '", SLFN,       "',"
+			SGiven, SLGiven, SMiddle, SLMiddle, SNickname,
-	"        '", SFamily,   "', '", SLFamily,   "',"
+			SLNickname, SBDay, SLBDay, SCTRY, SLCTRY,
-	"        '", SGiven,    "', '", SLGiven,    "',"
+			SLocality, SLLocality, SEMail, SLEMail, SOrgName,
-	"        '", SMiddle,   "', '", SLMiddle,   "',"
+			SLOrgName, SOrgUnit, SLOrgUnit],
-	"        '", SNickname, "', '", SLNickname, "',"
+		       ["lusername='", LUsername, "'"])
-	"        '", SBDay,     "', '", SLBDay,	   "',"
+      end).
-	"        '", SCTRY,     "', '", SLCTRY,	   "',"
-	"        '", SLocality, "', '", SLLocality, "',"
-	"        '", SEMail,    "', '", SLEMail,	   "',"
-	"        '", SOrgName,  "', '", SLOrgName,  "',"
-	"        '", SOrgUnit,  "', '", SLOrgUnit,  "');"]]).
 
 get_vcard(LServer, Username) ->
     ejabberd_odbc:sql_query(
@@ -416,6 +437,103 @@ get_vcard(LServer, Username) ->
       ["select vcard from vcard "
        "where username='", Username, "';"]).
 
+get_default_privacy_list(LServer, Username) ->
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["select name from privacy_default_list "
+       "where username='", Username, "';"]).
+
+get_default_privacy_list_t(Username) ->
+    ejabberd_odbc:sql_query_t(
+      ["select name from privacy_default_list "
+       "where username='", Username, "';"]).
+
+get_privacy_list_names(LServer, Username) ->
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["select name from privacy_list "
+       "where username='", Username, "';"]).
+
+get_privacy_list_names_t(Username) ->
+    ejabberd_odbc:sql_query_t(
+      ["select name from privacy_list "
+       "where username='", Username, "';"]).
+
+get_privacy_list_id(LServer, Username, SName) ->
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["select id from privacy_list "
+       "where username='", Username, "' and name='", SName, "';"]).
+
+get_privacy_list_id_t(Username, SName) ->
+    ejabberd_odbc:sql_query_t(
+      ["select id from privacy_list "
+       "where username='", Username, "' and name='", SName, "';"]).
+
+get_privacy_list_data(LServer, Username, SName) ->
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["select t, value, action, ord, match_all, match_iq, "
+       "match_message, match_presence_in, match_presence_out "
+       "from privacy_list_data "
+       "where id = (select id from privacy_list where "
+       "            username='", Username, "' and name='", SName, "') "
+       "order by ord;"]).
+
+get_privacy_list_data_by_id(LServer, ID) ->
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["select t, value, action, ord, match_all, match_iq, "
+       "match_message, match_presence_in, match_presence_out "
+       "from privacy_list_data "
+       "where id='", ID, "' order by ord;"]).
+
+set_default_privacy_list(Username, SName) ->
+    update_t("privacy_default_list", ["username", "name"],
+	     [Username, SName], ["username='", Username, "'"]).
+
+unset_default_privacy_list(LServer, Username) ->
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["delete from privacy_default_list "
+       "      where username='", Username, "';"]).
+
+remove_privacy_list(Username, SName) ->
+    ejabberd_odbc:sql_query_t(
+      ["delete from privacy_list "
+       "where username='", Username, "' and name='", SName, "';"]).
+
+add_privacy_list(Username, SName) ->
+    ejabberd_odbc:sql_query_t(
+      ["insert into privacy_list(username, name) "
+       "values ('", Username, "', '", SName, "');"]).
+
+set_privacy_list(ID, RItems) ->
+    ejabberd_odbc:sql_query_t(
+      ["delete from privacy_list_data "
+       "where id='", ID, "';"]),
+    lists:foreach(fun(Items) ->
+			  ejabberd_odbc:sql_query_t(
+			    ["insert into privacy_list_data("
+			     "id, t, value, action, ord, match_all, match_iq, "
+			     "match_message, match_presence_in, "
+			     "match_presence_out "
+			     ") "
+			     "values ('", ID, "', '",
+			     string:join(Items, "', '"), "');"])
+		  end, RItems).
+
+del_privacy_lists(LServer, Server, Username) ->
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["delete from privacy_list where username='", Username, "';"]),
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["delete from privacy_list_data where value='", Username++"@"++Server, "';"]),
+    ejabberd_odbc:sql_query(
+      LServer,
+      ["delete from privacy_default_list where username='", Username, "';"]).
+
 %% Characters to escape
 escape($\0) -> "\\0";
 escape($\n) -> "\\n";