pubsub improvement, fixes EJAB-684 EJAB-675 EJAB-663

SVN Revision: 1472

5 files changed, 77 insertions, 37 deletions

diff --git a/ChangeLog b/ChangeLog
index be5f8c04..5c9c0762 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2008-07-23  Christophe Romain  <christophe.romain@process-one.net>
+
+	* src/mod_pubsub/mod_pubsub.erl: remove_user hook removes
+	subscriptions (EJAB-684), send the last published and not the
+	first published item (EJAB-675), remove the pubsub/nodes tree.
+	* src/mod_pubsub/node_pep.erl: added acl and jid match on node
+	creation permission (EJAB-663)
+	* src/mod_pubsub/node_default.erl: fix node creation permission 
+	issue for service
+	* src/mod_pubsub/node_zoo.erl: Likewise
+
 2008-07-22  Badlop  <badlop@process-one.net>
 
 	* src/ejabberd_config.erl: If syntax mistake in config file, show
diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl
index e83e0170..a993af71 100644
--- a/src/mod_pubsub/mod_pubsub.erl
+++ b/src/mod_pubsub/mod_pubsub.erl
@@ -226,8 +226,6 @@ terminate_plugins(Host, ServerHost, Plugins, TreePlugin) ->
     ok.
 
 init_nodes(Host, ServerHost) ->
-    create_node(Host, ServerHost, ["pubsub"], service_jid(Host), ?STDNODE),
-    create_node(Host, ServerHost, ["pubsub", "nodes"], service_jid(Host), ?STDNODE),
     create_node(Host, ServerHost, ["home"], service_jid(Host), ?STDNODE),
     create_node(Host, ServerHost, ["home", ServerHost], service_jid(Host), ?STDNODE),
     ok.
@@ -416,7 +414,7 @@ remove_user(User, Server) ->
     LUser = jlib:nodeprep(User),
     LServer = jlib:nameprep(Server),
     Proc = gen_mod:get_module_proc(Server, ?PROCNAME),
-    gen_server:cast(Proc, {remove, LUser, LServer}).
+    gen_server:cast(Proc, {remove_user, LUser, LServer}).
 
 %%--------------------------------------------------------------------
 %% Function:
@@ -513,10 +511,27 @@ handle_cast({presence, JID, Pid}, State) ->
     end,
     {noreply, State};
 
-handle_cast({remove, User, Server}, State) ->
-    Owner = jlib:make_jid(User, Server, ""),
-    delete_nodes(Server, Owner),
-    {noreply, State};
+handle_cast({remove_user, User, Host}, State) ->
+    Owner = jlib:make_jid(User, Host, ""),
+    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
+    %% remove user's subscriptions
+    lists:foreach(fun(Type) ->
+	{result, Subscriptions} = node_action(Type, get_entity_subscriptions, [Host, Owner]),
+	lists:foreach(fun
+	    ({Node, subscribed}) ->
+		JID = jlib:jid_to_string(Owner),
+		unsubscribe_node(Host, Node, Owner, JID, all);
+	    (_) ->  
+		ok
+	end, Subscriptions)
+    end, State#state.plugins),
+    %% remove user's PEP nodes 
+    lists:foreach(fun(#pubsub_node{nodeid={NodeKey, NodeName}}) ->
+	delete_node(NodeKey, NodeName, Owner)
+    end, tree_action(Host, get_nodes, [OwnerKey])),
+    %% remove user's nodes
+    delete_node(Host, ["home", Host, User], Owner),
+    {noreply, State}; 
 
 handle_cast(_Msg, State) ->
     {noreply, State}.
@@ -1295,12 +1310,6 @@ delete_node(Host, Node, Owner) ->
 	{result, Result} ->
 	    {result, Result}
     end.
-delete_nodes(Host, Owner) ->
-    %% This removes only PEP nodes when user is removed
-    OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)),
-    lists:foreach(fun(#pubsub_node{nodeid={NodeKey, NodeName}}) ->
-	delete_node(NodeKey, NodeName, Owner)
-    end, tree_action(Host, get_nodes, [OwnerKey])).
 
 %% @spec (Host, Node, From, JID) ->
 %%		  {error, Reason::stanzaError()} |
@@ -1725,9 +1734,9 @@ send_last_item(Host, Node, LJID) ->
 send_items(Host, Node, LJID, Number) ->
     Items = get_items(Host, Node),
     ToSend = case Number of
-		 last -> lists:sublist(Items, 1);
+		 last -> hd(lists:reverse(Items));
 		 all -> Items;
-		 N when N > 0 -> lists:sublist(Items, N);
+		 N when N > 0 -> lists:nthtail(length(Items)-N, Items);
 		 _ -> Items
 	     end,
     ItemsEls = lists:map(
@@ -2414,9 +2423,11 @@ get_default(Host, _Node, _From, Lang) ->
 %% The result depend of the node type plugin system.
 get_option([], _) -> false;
 get_option(Options, Var) ->
+    get_option(Options, Var, false).
+get_option(Options, Var, Def) ->
     case lists:keysearch(Var, 1, Options) of
 	{value, {_Val, Ret}} -> Ret;
-	_ -> false
+	_ -> Def
     end.
 
 %% Get default options from the module plugin.
@@ -2461,7 +2472,7 @@ max_items(Options) ->
 
 -define(STRING_CONFIG_FIELD(Label, Var),
 	?STRINGXFIELD(Label, "pubsub#" ++ atom_to_list(Var),
-		      get_option(Options, Var))).
+		      get_option(Options, Var, ""))).
 
 -define(INTEGER_CONFIG_FIELD(Label, Var),
 	?STRINGXFIELD(Label, "pubsub#" ++ atom_to_list(Var),
diff --git a/src/mod_pubsub/node_default.erl b/src/mod_pubsub/node_default.erl
index 49167626..af8ad9dc 100644
--- a/src/mod_pubsub/node_default.erl
+++ b/src/mod_pubsub/node_default.erl
@@ -193,21 +193,23 @@ features() ->
 %% module by implementing this function like this:
 %% ```check_create_user_permission(Host, Node, Owner, Access) ->
 %%	   node_default:check_create_user_permission(Host, Node, Owner, Access).'''</p>
-create_node_permission(_Host, ServerHost, Node, _ParentNode, Owner, Access) ->
+create_node_permission(Host, ServerHost, Node, _ParentNode, Owner, Access) ->
     LOwner = jlib:jid_tolower(Owner),
     {User, Server, _Resource} = LOwner,
-    Allowed = case acl:match_rule(ServerHost, Access, LOwner) of
+    Allowed = case LOwner of
+	{"", Host, ""} ->
+	    true; % pubsub service always allowed
+	_ ->
+	    case acl:match_rule(ServerHost, Access, LOwner) of
 		allow ->
 		    case Node of
 			["home", Server, User | _] -> true;
 			_ -> false
 		    end;
 		_ ->
-		    case Owner of
-			{jid, "", _, "", "", _, ""} -> true;
-			_ -> false
-		    end
-	    end,
+		    false
+	    end
+    end,
     {result, Allowed}.
 
 %% @spec (Host, Node, Owner) ->
diff --git a/src/mod_pubsub/node_pep.erl b/src/mod_pubsub/node_pep.erl
index 2c648148..c986ef61 100644
--- a/src/mod_pubsub/node_pep.erl
+++ b/src/mod_pubsub/node_pep.erl
@@ -110,10 +110,25 @@ features() ->
      "subscribe" %*
     ].
 
-create_node_permission(_Host, _ServerHost, _Node, _ParentNode, _Owner, _Access) ->
-    %% TODO may we check bare JID match ?
-    {result, true}.
-
+create_node_permission(Host, ServerHost, _Node, _ParentNode, Owner, Access) ->
+    LOwner = jlib:jid_tolower(Owner),
+    {User, Server, _Resource} = LOwner, 
+    Allowed = case LOwner of
+	{"", Host, ""} ->
+	    true; % pubsub service always allowed
+	_ ->
+	    case acl:match_rule(ServerHost, Access, LOwner) of
+		allow ->
+		    case Host of 
+			{User, Server, _} -> true;
+			_ -> false
+		    end;
+		_ ->
+		    false   
+	    end
+    end,
+    {result, Allowed}.
+    
 create_node(Host, Node, Owner) ->
     case node_default:create_node(Host, Node, Owner) of
 	{result, _} -> {result, []};
diff --git a/src/mod_pubsub/node_zoo.erl b/src/mod_pubsub/node_zoo.erl
index 45e601ed..520a006a 100644
--- a/src/mod_pubsub/node_zoo.erl
+++ b/src/mod_pubsub/node_zoo.erl
@@ -91,18 +91,19 @@ features() ->
 %% use same code as node_default, but do not limite node to
 %% the home/localhost/user/... hierarchy
 %% any node is allowed
-create_node_permission(_Host, ServerHost, _Node, _ParentNode, Owner, Access) ->
+create_node_permission(Host, ServerHost, _Node, _ParentNode, Owner, Access) ->
     LOwner = jlib:jid_tolower(Owner),
-    %%{_User, _Server, _Resource} = LOwner, 
-    Allowed = case acl:match_rule(ServerHost, Access, LOwner) of
+    Allowed = case LOwner of
+	{"", Host, ""} ->
+	    true; % pubsub service always allowed
+	_ ->
+	    case acl:match_rule(ServerHost, Access, LOwner) of
 		allow ->
 		    true;
-		_ ->    
-		    case Owner of
-		    {jid, "", _, "", "", _, ""} -> true;
-		    _ -> false
-		    end     
-		end,    
+		_ ->
+		    false
+	    end
+    end,
     {result, Allowed}.
 
 create_node(Host, Node, Owner) ->