several improvements, and fix odbc subscriptions issues

SVN Revision: 2553

8 files changed, 249 insertions, 163 deletions

diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl
index 3469173fe..017a3d2fa 100644
--- a/src/mod_pubsub/mod_pubsub.erl
+++ b/src/mod_pubsub/mod_pubsub.erl
@@ -1409,7 +1409,11 @@ send_pending_auth_events(Host, Node, Owner) ->
 		    true ->
 			case node_call(Type, get_affiliation, [NodeID, Owner]) of
 			    {result, owner} ->
-				broadcast_pending_auth_events(N),
+				{result, Subscriptions} = node_call(Type, get_node_subscriptions, [NodeID]),
+				lists:foreach(fun({J, pending, _SubID}) -> send_authorization_request(N, jlib:make_jid(J));
+						 ({J, pending}) -> send_authorization_request(N, jlib:make_jid(J));
+						 (_) -> ok
+					      end, Subscriptions),
 				{result, ok};
 			    _ ->
 				{error, ?ERR_FORBIDDEN}
@@ -1425,14 +1429,6 @@ send_pending_auth_events(Host, Node, Owner) ->
 	    Err
     end.
 
-broadcast_pending_auth_events(#pubsub_node{type = Type, id = NodeID} = Node) ->
-    {result, Subscriptions} = node_call(Type, get_node_subscriptions, [NodeID]),
-    lists:foreach(fun ({J, pending, _SubID}) ->
-			  send_authorization_request(Node, jlib:make_jid(J));
-		      ({J, pending}) ->
-			  send_authorization_request(Node, jlib:make_jid(J))
-		  end, Subscriptions).
-
 %%% authorization handling
 
 send_authorization_request(#pubsub_node{owners = Owners, nodeid = {Host, Node}}, Subscriber) ->
@@ -1762,18 +1758,22 @@ delete_node(_Host, [], _Owner) ->
     {error, ?ERR_NOT_ALLOWED};
 delete_node(Host, Node, Owner) ->
     Action = fun(#pubsub_node{type = Type, id = NodeId}) ->
-		     SubsByDepth = get_collection_subscriptions(Host, Node),
-		     case node_call(Type, get_affiliation, [NodeId, Owner]) of
-			 {result, owner} ->
-			     Removed = tree_call(Host, delete_node, [Host, Node]),
-			     case node_call(Type, delete_node, [Removed]) of
+		    SubsByDepth = get_collection_subscriptions(Host, Node),
+		    case node_call(Type, get_affiliation, [NodeId, Owner]) of
+			{result, owner} ->
+			    SubsByDepth = case tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]) of
+				{result, T} -> [{Depth, [{N, get_node_subs(N)} || N <- Nodes]} || {Depth, Nodes} <- T];
+				_ -> []
+			    end,
+			    Removed = tree_call(Host, delete_node, [Host, Node]),
+			    case node_call(Type, delete_node, [Removed]) of
 				{result, Res} -> {result, {SubsByDepth, Res}};
-				Error	 -> Error
-			     end;
-			 _ ->
-			     %% Entity is not an owner
-			     {error, ?ERR_FORBIDDEN}
-		     end
+				Error -> Error
+			    end;
+			_ ->
+			    %% Entity is not an owner
+			    {error, ?ERR_FORBIDDEN}
+		    end
 	     end,
     Reply = [],
     case transaction(Host, Node, Action, transaction) of
@@ -2959,24 +2959,28 @@ broadcast_config_notification(Host, Node, NodeId, Type, NodeOptions, Lang) ->
     end.
 
 get_collection_subscriptions(Host, Node) ->
-    lists:map(fun ({Depth, Nodes}) ->
-		{Depth, [{N, get_node_subs(N)} || N <- Nodes]}
-	    end, tree_action(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)])).
+    Action = fun() ->
+	    {result, lists:map(fun({Depth, Nodes}) ->
+			{Depth, [{N, get_node_subs(N)} || N <- Nodes]}
+	    end, tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]))}
+	end,
+    case transaction(Action, sync_dirty) of
+	{result, CollSubs} -> CollSubs;
+	_ -> []
+    end.
 
 get_node_subs(#pubsub_node{type   = Type,
-			   nodeid = {Host, Node},
 			   id     = NodeID}) ->
-    case node_action(Host, Type, get_node_subscriptions, [NodeID]) of
-	{result, Subs} ->
-	    get_options_for_subs(Host, Node, NodeID, Subs);
-	Other ->
-	    Other
+    case node_call(Type, get_node_subscriptions, [NodeID]) of
+	{result, Subs} -> get_options_for_subs(NodeID, Subs);
+	Other -> Other
     end.
 
-get_options_for_subs(_Host, Node, NodeID, Subs) ->
+get_options_for_subs(NodeID, Subs) ->
     lists:foldl(fun({JID, subscribed, SubID}, Acc) ->
-			case pubsub_subscription:get_subscription(JID, NodeID, SubID) of
-			    {result, #pubsub_subscription{options = Options}} -> [{JID, Node, Options} | Acc];
+			case pubsub_subscription:read_subscription(JID, NodeID, SubID) of
+			    {error, notfound} -> [{JID, SubID, []} | Acc];
+			    #pubsub_subscription{options = Options} -> [{JID, SubID, Options} | Acc];
 			    _ -> Acc
 			end;
 		    (_, Acc) ->
@@ -3006,8 +3010,7 @@ broadcast_stanza(Host, Node, _NodeId, _Type, NodeOptions, SubsByDepth, NotifyTyp
     BroadcastAll = get_option(NodeOptions, broadcast_all_resources), %% XXX this is not standard, but usefull
     From = service_jid(Host),
     %% Handles explicit subscriptions
-    FilteredSubsByDepth = depths_to_deliver(NotifyType, SubsByDepth),
-    NodesByJID = collate_subs_by_jid(FilteredSubsByDepth),
+    NodesByJID = subscribed_nodes_by_jid(NotifyType, SubsByDepth),
     lists:foreach(fun ({LJID, Nodes}) ->
 			  LJIDs = case BroadcastAll of
 				      true ->
@@ -3068,36 +3071,28 @@ broadcast_stanza(Host, Node, _NodeId, _Type, NodeOptions, SubsByDepth, NotifyTyp
 	    ok
     end.
 
-depths_to_deliver(NotifyType, SubsByDepth) ->
-    NodesToDeliver =
-	fun (Depth, Node, Subs, Acc) ->
-		lists:foldl(fun ({LJID, _Node, SubOptions} = S, Acc2) ->
+subscribed_nodes_by_jid(NotifyType, SubsByDepth) ->
+    NodesToDeliver = fun(Depth, Node, Subs, Acc) ->
+		NodeId = case Node#pubsub_node.nodeid of
+		    {_, N} -> N;
+		    Other -> Other
+		end,
+		NodeOptions = Node#pubsub_node.options,
+		lists:foldl(fun({LJID, _SubID, SubOptions}, Acc2) ->
 				     case is_to_deliver(LJID, NotifyType, Depth,
-							Node#pubsub_node.options,
-							SubOptions) of
-					 true  -> [S | Acc2];
+							NodeOptions, SubOptions) of
+					 true  -> [{LJID, NodeId}|Acc2];
 					 false -> Acc2
 				     end
 			     end, Acc, Subs)
 	end,
-
-    DepthsToDeliver =
-	fun ({Depth, SubsByNode}, Acc) ->
-		lists:foldl(fun ({Node, Subs}, Acc2) ->
+    DepthsToDeliver = fun({Depth, SubsByNode}, Acc) ->
+		lists:foldl(fun({Node, Subs}, Acc2) ->
 				    NodesToDeliver(Depth, Node, Subs, Acc2)
 			    end, Acc, SubsByNode)
 	end,
-
-    lists:foldl(DepthsToDeliver, [], SubsByDepth).
-
-collate_subs_by_jid(SubsByDepth) ->
-    lists:foldl(fun ({JID, Node, _Options}, Acc) ->
-			OldNodes = case lists:keysearch(JID, 1, Acc) of
-				       {value, {JID, Nodes}} -> Nodes;
-				       false		 -> []
-				   end,
-			lists:keystore(JID, 1, Acc, {JID, [Node | OldNodes]})
-		end, [], SubsByDepth).
+    JIDSubs = lists:foldl(DepthsToDeliver, [], SubsByDepth),
+    [{LJID, proplists:append_values(LJID, JIDSubs)} || LJID <- proplists:get_keys(JIDSubs)].
 
 %% If we don't know the resource, just pick first if any
 %% If no resource available, check if caps anyway (remote online)
diff --git a/src/mod_pubsub/mod_pubsub_odbc.erl b/src/mod_pubsub/mod_pubsub_odbc.erl
index e7d593438..08fe35a7d 100644
--- a/src/mod_pubsub/mod_pubsub_odbc.erl
+++ b/src/mod_pubsub/mod_pubsub_odbc.erl
@@ -1093,9 +1093,10 @@ iq_pubsub(Host, ServerHost, From, IQType, SubEl, Lang, Access, Plugins) ->
 
 iq_pubsub_owner(Host, ServerHost, From, IQType, SubEl, Lang) ->
     {xmlelement, _, _, SubEls} = SubEl,
-    Action = lists:filter(fun({xmlelement, "set", _, _}) -> false;
-			     (_) -> true
-			end, xml:remove_cdata(SubEls)),
+    NoRSM = lists:filter(fun({xmlelement, Name, _, _}) ->
+			    Name == "set"
+			end, SubEls),
+    Action = xml:remove_cdata(SubEls) -- NoRSM,
     case Action of
 	[{xmlelement, Name, Attrs, Els}] ->
 	    Node = case Host of
@@ -1240,7 +1241,11 @@ send_pending_auth_events(Host, Node, Owner) ->
 		    true ->
 			case node_call(Type, get_affiliation, [NodeID, Owner]) of
 			    {result, owner} ->
-				broadcast_pending_auth_events(N),
+				{result, Subscriptions} = node_call(Type, get_node_subscriptions, [NodeID]),
+				lists:foreach(fun({J, pending, _SubID}) -> send_authorization_request(N, jlib:make_jid(J));
+						 ({J, pending}) -> send_authorization_request(N, jlib:make_jid(J));
+						 (_) -> ok
+					      end, Subscriptions),
 				{result, ok};
 			    _ ->
 				{error, ?ERR_FORBIDDEN}
@@ -1256,14 +1261,6 @@ send_pending_auth_events(Host, Node, Owner) ->
 	    Err
     end.
 
-broadcast_pending_auth_events(#pubsub_node{type = Type, id = NodeID} = Node) ->
-    {result, Subscriptions} = node_call(Type, get_node_subscriptions, [NodeID]),
-    lists:foreach(fun ({J, pending, _SubID}) ->
-			  send_authorization_request(Node, jlib:make_jid(J));
-		      ({J, pending}) ->
-			  send_authorization_request(Node, jlib:make_jid(J))
-		  end, Subscriptions).
-
 %%% authorization handling
 
 send_authorization_request(#pubsub_node{nodeid = {Host, Node}, type = Type, id = NodeId}, Subscriber) ->
@@ -1593,18 +1590,19 @@ delete_node(_Host, [], _Owner) ->
     {error, ?ERR_NOT_ALLOWED};
 delete_node(Host, Node, Owner) ->
     Action = fun(#pubsub_node{type = Type, id = NodeId}) ->
-		     SubsByDepth = get_collection_subscriptions(Host, Node),
-		     case node_call(Type, get_affiliation, [NodeId, Owner]) of
-			 {result, owner} ->
-			     Removed = tree_call(Host, delete_node, [Host, Node]),
-			     case node_call(Type, delete_node, [Removed]) of
+		    case node_call(Type, get_affiliation, [NodeId, Owner]) of
+			{result, owner} ->
+			    ParentTree = tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]),
+			    SubsByDepth = [{Depth, [{N, get_node_subs(N)} || N <- Nodes]} || {Depth, Nodes} <- ParentTree],
+			    Removed = tree_call(Host, delete_node, [Host, Node]),
+			    case node_call(Type, delete_node, [Removed]) of
 				{result, Res} -> {result, {SubsByDepth, Res}};
-				Error	 -> Error
-			     end;
-			 _ ->
-			     %% Entity is not an owner
-			     {error, ?ERR_FORBIDDEN}
-		     end
+				Error -> Error
+			    end;
+			_ ->
+			    %% Entity is not an owner
+			    {error, ?ERR_FORBIDDEN}
+		    end
 	     end,
     Reply = [],
     case transaction(Host, Node, Action, transaction) of
@@ -1656,7 +1654,7 @@ delete_node(Host, Node, Owner) ->
 %%<li>The node does not exist.</li>
 %%</ul>
 subscribe_node(Host, Node, From, JID, Configuration) ->
-    {result, SubOpts} = pubsub_subscription:parse_options_xform(Configuration),
+    {result, SubOpts} = pubsub_subscription_odbc:parse_options_xform(Configuration),
     Subscriber = case jlib:string_to_jid(JID) of
 		     error -> {"", "", ""};
 		     J -> jlib:jid_tolower(J)
@@ -2289,11 +2287,11 @@ get_options_helper(JID, Lang, NodeID, SubID, Type) ->
     end.
 
 read_sub(Subscriber, NodeID, SubID, Lang) ->
-    case pubsub_subscription:get_subscription(Subscriber, NodeID, SubID) of
+    case pubsub_subscription_odbc:get_subscription(Subscriber, NodeID, SubID) of
 	{error, notfound} ->
 	    {error, ?ERR_EXTENDED(?ERR_NOT_ACCEPTABLE, "invalid-subid")};
 	{result, #pubsub_subscription{options = Options}} ->
-	    pubsub_subscription:get_options_xform(Lang, Options)
+	    pubsub_subscription_odbc:get_options_xform(Lang, Options)
     end.
 
 set_options(Host, Node, JID, SubID, Configuration) ->
@@ -2318,7 +2316,7 @@ set_options_helper(Configuration, JID, NodeID, SubID, Type) ->
 		     error -> {"", "", ""};
 		     J -> jlib:jid_tolower(J)
 		 end,
-    {result, SubOpts} = pubsub_subscription:parse_options_xform(Configuration),
+    {result, SubOpts} = pubsub_subscription_odbc:parse_options_xform(Configuration),
     {result, Subs} = node_call(Type, get_subscriptions,
 			       [NodeID, Subscriber]),
     SubIDs = lists:foldl(fun({subscribed, SID}, Acc) ->
@@ -2338,7 +2336,7 @@ set_options_helper(Configuration, JID, NodeID, SubID, Type) ->
     end.
 
 write_sub(Subscriber, NodeID, SubID, Options) ->
-    case pubsub_subscription:set_subscription(Subscriber, NodeID, SubID,
+    case pubsub_subscription_odbc:set_subscription(Subscriber, NodeID, SubID,
 					      Options) of
 	{error, notfound} ->
 	    {error, ?ERR_EXTENDED(?ERR_NOT_ACCEPTABLE, "invalid-subid")};
@@ -2785,24 +2783,28 @@ broadcast_config_notification(Host, Node, NodeId, Type, NodeOptions, Lang) ->
     end.
 
 get_collection_subscriptions(Host, Node) ->
-    lists:map(fun ({Depth, Nodes}) ->
-		{Depth, [{N, get_node_subs(N)} || N <- Nodes]}
-	    end, tree_action(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)])).
+    Action = fun() ->
+	    {result, lists:map(fun({Depth, Nodes}) ->
+			{Depth, [{N, get_node_subs(N)} || N <- Nodes]}
+	    end, tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]))}
+	end,
+    case transaction(Host, Action, sync_dirty) of
+	{result, CollSubs} -> CollSubs;
+	_ -> []
+    end.
 
 get_node_subs(#pubsub_node{type   = Type,
-			   nodeid = {Host, Node},
 			   id     = NodeID}) ->
-    case node_action(Host, Type, get_node_subscriptions, [NodeID]) of
-	{result, Subs} ->
-	    get_options_for_subs(Host, Node, NodeID, Subs);
-	Other ->
-	    Other
+    case node_call(Type, get_node_subscriptions, [NodeID]) of
+	{result, Subs} -> get_options_for_subs(NodeID, Subs);
+	Other -> Other
     end.
 
-get_options_for_subs(_Host, Node, NodeID, Subs) ->
+get_options_for_subs(NodeID, Subs) ->
     lists:foldl(fun({JID, subscribed, SubID}, Acc) ->
-			case pubsub_subscription:get_subscription(JID, NodeID, SubID) of
-			    {result, #pubsub_subscription{options = Options}} -> [{JID, Node, Options} | Acc];
+			case pubsub_subscription_odbc:get_subscription(JID, NodeID, SubID) of
+			    {error, notfound} -> [{JID, SubID, []} | Acc];
+			    {result, #pubsub_subscription{options = Options}} -> [{JID, SubID, Options} | Acc];
 			    _ -> Acc
 			end;
 		    (_, Acc) ->
@@ -2832,8 +2834,7 @@ broadcast_stanza(Host, Node, _NodeId, _Type, NodeOptions, SubsByDepth, NotifyTyp
     BroadcastAll = get_option(NodeOptions, broadcast_all_resources), %% XXX this is not standard, but usefull
     From = service_jid(Host),
     %% Handles explicit subscriptions
-    FilteredSubsByDepth = depths_to_deliver(NotifyType, SubsByDepth),
-    NodesByJID = collate_subs_by_jid(FilteredSubsByDepth),
+    NodesByJID = subscribed_nodes_by_jid(NotifyType, SubsByDepth),
     lists:foreach(fun ({LJID, Nodes}) ->
 			  LJIDs = case BroadcastAll of
 				      true ->
@@ -2894,36 +2895,28 @@ broadcast_stanza(Host, Node, _NodeId, _Type, NodeOptions, SubsByDepth, NotifyTyp
 	    ok
     end.
 
-depths_to_deliver(NotifyType, SubsByDepth) ->
-    NodesToDeliver =
-	fun (Depth, Node, Subs, Acc) ->
-		lists:foldl(fun ({LJID, _Node, SubOptions} = S, Acc2) ->
+subscribed_nodes_by_jid(NotifyType, SubsByDepth) ->
+    NodesToDeliver = fun(Depth, Node, Subs, Acc) ->
+		NodeId = case Node#pubsub_node.nodeid of
+		    {_, N} -> N;
+		    Other -> Other
+		end,
+		NodeOptions = Node#pubsub_node.options,
+		lists:foldl(fun({LJID, _SubID, SubOptions}, Acc2) ->
 				     case is_to_deliver(LJID, NotifyType, Depth,
-							Node#pubsub_node.options,
-							SubOptions) of
-					 true  -> [S | Acc2];
+							NodeOptions, SubOptions) of
+					 true  -> [{LJID, NodeId}|Acc2];
 					 false -> Acc2
 				     end
 			     end, Acc, Subs)
 	end,
-
-    DepthsToDeliver =
-	fun ({Depth, SubsByNode}, Acc) ->
-		lists:foldl(fun ({Node, Subs}, Acc2) ->
+    DepthsToDeliver = fun({Depth, SubsByNode}, Acc) ->
+		lists:foldl(fun({Node, Subs}, Acc2) ->
 				    NodesToDeliver(Depth, Node, Subs, Acc2)
 			    end, Acc, SubsByNode)
 	end,
-
-    lists:foldl(DepthsToDeliver, [], SubsByDepth).
-
-collate_subs_by_jid(SubsByDepth) ->
-    lists:foldl(fun ({JID, Node, _Options}, Acc) ->
-			OldNodes = case lists:keysearch(JID, 1, Acc) of
-				       {value, {JID, Nodes}} -> Nodes;
-				       false		 -> []
-				   end,
-			lists:keystore(JID, 1, Acc, {JID, [Node | OldNodes]})
-		end, [], SubsByDepth).
+    JIDSubs = lists:foldl(DepthsToDeliver, [], SubsByDepth),
+    [{LJID, proplists:append_values(LJID, JIDSubs)} || LJID <- proplists:get_keys(JIDSubs)].
 
 %% If we don't know the resource, just pick first if any
 %% If no resource available, check if caps anyway (remote online)
diff --git a/src/mod_pubsub/node_flat_odbc.erl b/src/mod_pubsub/node_flat_odbc.erl
index 9b8b49ad1..8c9032fe3 100644
--- a/src/mod_pubsub/node_flat_odbc.erl
+++ b/src/mod_pubsub/node_flat_odbc.erl
@@ -62,7 +62,8 @@
 	 get_item/7,
 	 get_item/2,
 	 set_item/1,
-	 get_item_name/3
+	 get_item_name/3,
+	 get_last_items/3
 	]).
 
 
@@ -190,3 +191,7 @@ set_item(Item) ->
 
 get_item_name(Host, Node, Id) ->
     node_hometree_odbc:get_item_name(Host, Node, Id).
+
+get_last_items(NodeId, From, Count) ->
+    node_hometree_odbc:get_last_items(NodeId, From, Count).
+
diff --git a/src/mod_pubsub/node_hometree_odbc.erl b/src/mod_pubsub/node_hometree_odbc.erl
index a9a815f57..514e7d959 100644
--- a/src/mod_pubsub/node_hometree_odbc.erl
+++ b/src/mod_pubsub/node_hometree_odbc.erl
@@ -392,8 +392,7 @@ unsubscribe_node(NodeId, Sender, Subscriber, SubId) ->
 		    delete_subscription(SubKey, NodeId, S, Affiliation, Subscriptions),
 		    {result, default};
 		false ->
-		    {error, ?ERR_EXTENDED(?ERR_UNEXPECTED_REQUEST,
-					  "not-subscribed")}
+		    {error, ?ERR_EXTENDED(?ERR_UNEXPECTED_REQUEST, "not-subscribed")}
 	    end;
 	%% No subid supplied, but there's only one matching
 	%% subscription, so use that.
@@ -702,10 +701,18 @@ get_entity_subscriptions_for_send_last(Host, Owner) ->
     end,
     Reply = case catch ejabberd_odbc:sql_query_t(Query) of
 	{selected, ["node", "type", "nodeid", "jid", "subscriptions"], RItems} ->
-	    lists:map(fun({N, T, I, J, S}) ->
+	    lists:foldl(fun({N, T, I, J, S}, Acc) ->
 		Node = nodetree_tree_odbc:raw_to_node(Host, {N, "", T, I}),
-		{Node, decode_subscriptions(S), decode_jid(J)}
-	    end, RItems);
+		Jid = decode_jid(J),
+		case decode_subscriptions(S) of
+		    [] ->
+			[{Node, none, Jid}|Acc];
+		    Subs ->
+			lists:foldl(fun({Sub, SubId}, Acc2) -> [{Node, Sub, SubId, Jid}|Acc2];
+				       (Sub, Acc2) -> [{Node, Sub, Jid}|Acc2]
+			end, Acc, Subs)
+		end
+	    end, [], RItems);
 	_ ->
 	    []
 	end,
@@ -717,8 +724,17 @@ get_node_subscriptions(NodeId) ->
 		  "from pubsub_state "
 		  "where nodeid='", NodeId, "';"]) of
 	    {selected, ["jid", "subscriptions"], RItems} ->
-		lists:map(fun({J, S}) -> {decode_jid(J), decode_subscriptions(S)} end, RItems);
-    % TODO {J, S, SubId}
+		lists:foldl(fun({J, S}, Acc) ->
+		    Jid = decode_jid(J),
+		    case decode_subscriptions(S) of
+			[] ->
+			    [{Jid, none}|Acc];
+			Subs ->
+			    lists:foldl(fun({Sub, SubId}, Acc2) -> [{Jid, Sub, SubId}|Acc2];
+					   (Sub, Acc2) -> [{Jid, Sub}|Acc2]
+			    end, Acc, Subs)
+		    end
+		end, [], RItems);
 	    _ ->
 		[]
     end,
@@ -731,7 +747,7 @@ get_subscriptions(NodeId, Owner) ->
 		 ["select subscriptions from pubsub_state "
 		  "where nodeid='", NodeId, "' and jid='", J, "';"]) of
 	{selected, ["subscriptions"], [{S}]} -> decode_subscriptions(S);
-	_ -> none
+	_ -> []
     end,
     {result, Reply}.
 
@@ -1223,7 +1239,7 @@ select_affiliation_subscriptions(NodeId, JID) ->
 	{selected, ["affiliation", "subscriptions"], [{A, S}]} ->
 	    {decode_affiliation(A), decode_subscriptions(S)};
 	_ ->
-	    {none, none}
+	    {none, []}
     end.
 select_affiliation_subscriptions(NodeId, JID, JID) ->
     select_affiliation_subscriptions(NodeId, JID);
diff --git a/src/mod_pubsub/nodetree_tree.erl b/src/mod_pubsub/nodetree_tree.erl
index 99b4b8891..ba8168f52 100644
--- a/src/mod_pubsub/nodetree_tree.erl
+++ b/src/mod_pubsub/nodetree_tree.erl
@@ -151,7 +151,7 @@ get_parentnodes(_Host, _Node, _From) ->
 get_parentnodes_tree(Host, Node, From) ->
     case get_node(Host, Node, From) of
 	N when is_record(N, pubsub_node) -> [{0, [N]}];
-	Error -> Error
+	_Error -> []
     end.
 
 %% @spec (Host, Node, From) -> [pubsubNode()] | {error, Reason}
diff --git a/src/mod_pubsub/nodetree_tree_odbc.erl b/src/mod_pubsub/nodetree_tree_odbc.erl
index 6468913d1..04b098fcb 100644
--- a/src/mod_pubsub/nodetree_tree_odbc.erl
+++ b/src/mod_pubsub/nodetree_tree_odbc.erl
@@ -88,13 +88,11 @@ options() ->
     [{virtual_tree, false},
      {odbc, true}].
 
-
-get_node(Host, Node, _From) ->
-    get_node(Host, Node).
-
 %% @spec (Host, Node) -> pubsubNode() | {error, Reason}
 %%     Host = mod_pubsub:host()
 %%     Node = mod_pubsub:pubsubNode()
+get_node(Host, Node, _From) ->
+    get_node(Host, Node).
 get_node(Host, Node) ->
     H = ?PUBSUB:escape(Host),
     N = ?PUBSUB:escape(?PUBSUB:node_to_string(Node)),
@@ -124,10 +122,10 @@ get_node(NodeId) ->
 	    {error, ?ERR_ITEM_NOT_FOUND}
     end.
 
-get_nodes(Host, _From) ->
-    get_nodes(Host).
 %% @spec (Host) -> [pubsubNode()] | {error, Reason}
 %%	 Host = mod_pubsub:host() | mod_pubsub:jid()
+get_nodes(Host, _From) ->
+    get_nodes(Host).
 get_nodes(Host) ->
     H = ?PUBSUB:escape(Host),
     case catch ejabberd_odbc:sql_query_t(
@@ -162,7 +160,7 @@ get_parentnodes(_Host, _Node, _From) ->
 get_parentnodes_tree(Host, Node, From) ->
     case get_node(Host, Node, From) of
 	N when is_record(N, pubsub_node) -> [{0, [N]}];
-	Error -> Error
+	_Error -> []
     end.
 
 get_subnodes(Host, Node, _From) ->
diff --git a/src/mod_pubsub/pubsub_odbc.patch b/src/mod_pubsub/pubsub_odbc.patch
index 0694e8302..a1426b0a2 100644
--- a/src/mod_pubsub/pubsub_odbc.patch
+++ b/src/mod_pubsub/pubsub_odbc.patch
@@ -1,5 +1,5 @@
---- mod_pubsub.erl	2009-08-25 18:29:16.000000000 +0200
-+++ mod_pubsub_odbc.erl	2009-08-27 10:51:06.000000000 +0200
+--- mod_pubsub.erl	2009-08-27 23:21:24.000000000 +0200
++++ mod_pubsub_odbc.erl	2009-08-27 23:36:47.000000000 +0200
 @@ -45,7 +45,7 @@
  %%% TODO
  %%% plugin: generate Reply (do not use broadcast atom anymore)
@@ -377,7 +377,7 @@
  		     sync_dirty) of
  	{result, Res} -> Res;
  	Err	   -> Err
-@@ -1435,7 +1267,7 @@
+@@ -1431,7 +1263,7 @@
  
  %%% authorization handling
  
@@ -386,7 +386,7 @@
      Lang = "en", %% TODO fix
      Stanza = {xmlelement, "message",
  	      [],
-@@ -1464,7 +1296,7 @@
+@@ -1460,7 +1292,7 @@
  		  [{xmlelement, "value", [], [{xmlcdata, "false"}]}]}]}]},
      lists:foreach(fun(Owner) ->
  	ejabberd_router ! {route, service_jid(Host), jlib:make_jid(Owner), Stanza}
@@ -395,7 +395,7 @@
  
  find_authorization_response(Packet) ->
      {xmlelement, _Name, _Attrs, Els} = Packet,
-@@ -1531,8 +1363,8 @@
+@@ -1527,8 +1359,8 @@
  			"true" -> true;
  			_ -> false
  		    end,
@@ -406,7 +406,7 @@
  			     {result, Subscriptions} = node_call(Type, get_subscriptions, [NodeId, Subscriber]),
  			     if
  				 not IsApprover ->
-@@ -1718,7 +1550,7 @@
+@@ -1714,7 +1546,7 @@
  	    Reply = [{xmlelement, "pubsub", [{"xmlns", ?NS_PUBSUB}],
  		      [{xmlelement, "create", nodeAttr(Node),
  			[]}]}],
@@ -415,7 +415,29 @@
  		{result, {Result, broadcast}} ->
  		    %%Lang = "en", %% TODO: fix
  		    %%OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-@@ -1830,7 +1662,7 @@
+@@ -1758,13 +1590,10 @@
+     {error, ?ERR_NOT_ALLOWED};
+ delete_node(Host, Node, Owner) ->
+     Action = fun(#pubsub_node{type = Type, id = NodeId}) ->
+-		    SubsByDepth = get_collection_subscriptions(Host, Node),
+ 		    case node_call(Type, get_affiliation, [NodeId, Owner]) of
+ 			{result, owner} ->
+-			    SubsByDepth = case tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]) of
+-				{result, T} -> [{Depth, [{N, get_node_subs(N)} || N <- Nodes]} || {Depth, Nodes} <- T];
+-				_ -> []
+-			    end,
++			    ParentTree = tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]),
++			    SubsByDepth = [{Depth, [{N, get_node_subs(N)} || N <- Nodes]} || {Depth, Nodes} <- ParentTree],
+ 			    Removed = tree_call(Host, delete_node, [Host, Node]),
+ 			    case node_call(Type, delete_node, [Removed]) of
+ 				{result, Res} -> {result, {SubsByDepth, Res}};
+@@ -1825,12 +1654,12 @@
+ %%<li>The node does not exist.</li>
+ %%</ul>
+ subscribe_node(Host, Node, From, JID, Configuration) ->
+-    {result, SubOpts} = pubsub_subscription:parse_options_xform(Configuration),
++    {result, SubOpts} = pubsub_subscription_odbc:parse_options_xform(Configuration),
+     Subscriber = case jlib:string_to_jid(JID) of
  		     error -> {"", "", ""};
  		     J -> jlib:jid_tolower(J)
  		 end,
@@ -424,7 +446,7 @@
  		    Features = features(Type),
  		    SubscribeFeature = lists:member("subscribe", Features),
  		    OptionsFeature = lists:member("subscription-options", Features),
-@@ -1849,9 +1681,13 @@
+@@ -1849,9 +1678,13 @@
  				    {"", "", ""} ->
  					{false, false};
  				    _ ->
@@ -441,7 +463,7 @@
  				end
  			end,
  		    if
-@@ -2174,7 +2010,7 @@
+@@ -2174,7 +2007,7 @@
  %% <p>The permission are not checked in this function.</p>
  %% @todo We probably need to check that the user doing the query has the right
  %% to read the items.
@@ -450,7 +472,7 @@
      MaxItems =
  	if
  	    SMaxItems == "" -> ?MAXITEMS;
-@@ -2213,11 +2049,11 @@
+@@ -2213,11 +2046,11 @@
  			     node_call(Type, get_items,
  				       [NodeId, From,
  					AccessModel, PresenceSubscription, RosterGroup,
@@ -464,7 +486,7 @@
  		    SendItems = case ItemIDs of
  			[] -> 
  			    Items;
-@@ -2230,7 +2066,8 @@
+@@ -2230,7 +2063,8 @@
  		    %% number of items sent to MaxItems:
  		    {result, [{xmlelement, "pubsub", [{"xmlns", ?NS_PUBSUB}],
  				[{xmlelement, "items", nodeAttr(Node),
@@ -474,7 +496,7 @@
  		Error ->
  		    Error
  	    end
-@@ -2262,15 +2099,22 @@
+@@ -2262,15 +2096,22 @@
  %% @doc <p>Resend the items of a node to the user.</p>
  %% @todo use cache-last-item feature
  send_items(Host, Node, NodeId, Type, LJID, last) ->
@@ -503,7 +525,7 @@
  send_items(Host, Node, NodeId, Type, LJID, Number) ->
      ToSend = case node_action(Host, Type, get_items, [NodeId, LJID]) of
  	{result, []} -> 
-@@ -2388,29 +2232,12 @@
+@@ -2388,29 +2229,12 @@
  	error ->
  	    {error, ?ERR_BAD_REQUEST};
  	_ ->
@@ -536,7 +558,39 @@
  				    end, Entities),
  				{result, []};
  			    _ ->
-@@ -2681,8 +2508,8 @@
+@@ -2463,11 +2287,11 @@
+     end.
+ 
+ read_sub(Subscriber, NodeID, SubID, Lang) ->
+-    case pubsub_subscription:get_subscription(Subscriber, NodeID, SubID) of
++    case pubsub_subscription_odbc:get_subscription(Subscriber, NodeID, SubID) of
+ 	{error, notfound} ->
+ 	    {error, ?ERR_EXTENDED(?ERR_NOT_ACCEPTABLE, "invalid-subid")};
+ 	{result, #pubsub_subscription{options = Options}} ->
+-	    pubsub_subscription:get_options_xform(Lang, Options)
++	    pubsub_subscription_odbc:get_options_xform(Lang, Options)
+     end.
+ 
+ set_options(Host, Node, JID, SubID, Configuration) ->
+@@ -2492,7 +2316,7 @@
+ 		     error -> {"", "", ""};
+ 		     J -> jlib:jid_tolower(J)
+ 		 end,
+-    {result, SubOpts} = pubsub_subscription:parse_options_xform(Configuration),
++    {result, SubOpts} = pubsub_subscription_odbc:parse_options_xform(Configuration),
+     {result, Subs} = node_call(Type, get_subscriptions,
+ 			       [NodeID, Subscriber]),
+     SubIDs = lists:foldl(fun({subscribed, SID}, Acc) ->
+@@ -2512,7 +2336,7 @@
+     end.
+ 
+ write_sub(Subscriber, NodeID, SubID, Options) ->
+-    case pubsub_subscription:set_subscription(Subscriber, NodeID, SubID,
++    case pubsub_subscription_odbc:set_subscription(Subscriber, NodeID, SubID,
+ 					      Options) of
+ 	{error, notfound} ->
+ 	    {error, ?ERR_EXTENDED(?ERR_NOT_ACCEPTABLE, "invalid-subid")};
+@@ -2681,8 +2505,8 @@
  				     {"subscription", subscription_to_string(Sub)} | nodeAttr(Node)], []}]}]},
  		ejabberd_router ! {route, service_jid(Host), jlib:make_jid(JID), Stanza}
  	    end,
@@ -547,7 +601,28 @@
  				true ->
  				    Result = lists:foldl(fun({JID, Subscription, SubId}, Acc) ->
  
-@@ -3179,6 +3006,30 @@
+@@ -2964,7 +2788,7 @@
+ 			{Depth, [{N, get_node_subs(N)} || N <- Nodes]}
+ 	    end, tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]))}
+ 	end,
+-    case transaction(Action, sync_dirty) of
++    case transaction(Host, Action, sync_dirty) of
+ 	{result, CollSubs} -> CollSubs;
+ 	_ -> []
+     end.
+@@ -2978,9 +2802,9 @@
+ 
+ get_options_for_subs(NodeID, Subs) ->
+     lists:foldl(fun({JID, subscribed, SubID}, Acc) ->
+-			case pubsub_subscription:read_subscription(JID, NodeID, SubID) of
++			case pubsub_subscription_odbc:get_subscription(JID, NodeID, SubID) of
+ 			    {error, notfound} -> [{JID, SubID, []} | Acc];
+-			    #pubsub_subscription{options = Options} -> [{JID, SubID, Options} | Acc];
++			    {result, #pubsub_subscription{options = Options}} -> [{JID, SubID, Options} | Acc];
+ 			    _ -> Acc
+ 			end;
+ 		    (_, Acc) ->
+@@ -3174,6 +2998,30 @@
  	    Result
      end.
  
@@ -578,7 +653,7 @@
  %% @spec (Host, Options) -> MaxItems
  %%	 Host = host()
  %%	 Options = [Option]
-@@ -3552,7 +3403,13 @@
+@@ -3547,7 +3395,13 @@
  tree_action(Host, Function, Args) ->
      ?DEBUG("tree_action ~p ~p ~p",[Host,Function,Args]),
      Fun = fun() -> tree_call(Host, Function, Args) end,
@@ -593,7 +668,7 @@
  
  %% @doc <p>node plugin call.</p>
  node_call(Type, Function, Args) ->
-@@ -3572,13 +3429,13 @@
+@@ -3567,13 +3421,13 @@
  
  node_action(Host, Type, Function, Args) ->
      ?DEBUG("node_action ~p ~p ~p ~p",[Host,Type,Function,Args]),
@@ -609,7 +684,7 @@
  			case tree_call(Host, get_node, [Host, Node]) of
  			    N when is_record(N, pubsub_node) ->
  				case Action(N) of
-@@ -3591,8 +3448,14 @@
+@@ -3586,8 +3440,14 @@
  			end
  		end, Trans).
  
@@ -626,7 +701,7 @@
  	{result, Result} -> {result, Result};
  	{error, Error} -> {error, Error};
  	{atomic, {result, Result}} -> {result, Result};
-@@ -3600,6 +3463,15 @@
+@@ -3595,6 +3455,15 @@
  	{aborted, Reason} ->
  	    ?ERROR_MSG("transaction return internal error: ~p~n", [{aborted, Reason}]),
  	    {error, ?ERR_INTERNAL_SERVER_ERROR};
@@ -642,7 +717,7 @@
  	{'EXIT', Reason} ->
  	    ?ERROR_MSG("transaction return internal error: ~p~n", [{'EXIT', Reason}]),
  	    {error, ?ERR_INTERNAL_SERVER_ERROR};
-@@ -3608,6 +3480,17 @@
+@@ -3603,6 +3472,17 @@
  	    {error, ?ERR_INTERNAL_SERVER_ERROR}
      end.
  
diff --git a/src/mod_pubsub/pubsub_subscription_odbc.erl b/src/mod_pubsub/pubsub_subscription_odbc.erl
index 98e62abaa..eebb7d838 100644
--- a/src/mod_pubsub/pubsub_subscription_odbc.erl
+++ b/src/mod_pubsub/pubsub_subscription_odbc.erl
@@ -88,14 +88,18 @@ init() ->
 
 subscribe_node(_JID, _NodeID, Options) ->
     SubId = make_subid(),
-    ok = ?DB_MOD:add_subscription(#pubsub_subscription{subid = SubId, options = Options}),
+    ?DB_MOD:add_subscription(#pubsub_subscription{subid = SubId, options = Options}),
     {result, SubId}.
 
 
 unsubscribe_node(_JID, _NodeID, SubID) ->
-    {ok, Sub} = ?DB_MOD:read_subscription(SubID),
-    ok = ?DB_MOD:delete_subscription(SubID),
-    {result, Sub}.
+    case ?DB_MOD:read_subscription(SubID) of
+	{ok, Sub} ->
+	    ?DB_MOD:delete_subscription(SubID),
+	    {result, Sub};
+	notfound ->
+	    {error, notfound}
+    end.
 
 get_subscription(_JID, _NodeID, SubID) ->
     case ?DB_MOD:read_subscription(SubID) of
@@ -107,7 +111,7 @@ get_subscription(_JID, _NodeID, SubID) ->
 set_subscription(_JID, _NodeID, SubID, Options) ->
     case ?DB_MOD:read_subscription(SubID) of
 	{ok, _} ->
-	     ok = ?DB_MOD:update_subscription(#pubsub_subscription{subid = SubID, options = Options}),
+	     ?DB_MOD:update_subscription(#pubsub_subscription{subid = SubID, options = Options}),
 	     {result, ok};
 	notfound -> 
 	     {error, notfound}