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

SVN Revision: 1472

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

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,9 +511,26 @@ handle_cast({presence, JID, Pid}, State) ->
     end,
     {noreply, State};
 
-handle_cast({remove, User, Server}, State) ->
+handle_cast({remove_user, User, Host}, State) ->
-    Owner = jlib:make_jid(User, Server, ""),
+    Owner = jlib:make_jid(User, Host, ""),
-    delete_nodes(Server, Owner),
+    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) ->
@@ -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),

src/mod_pubsub/node_default.erl

@@ -193,19 +193,21 @@ 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
+		    false
-			{jid, "", _, "", "", _, ""} -> true;
-			_ -> false
 	    end
     end,
     {result, Allowed}.

src/mod_pubsub/node_pep.erl

@@ -110,9 +110,24 @@ features() ->
      "subscribe" %*
     ].
 
-create_node_permission(_Host, _ServerHost, _Node, _ParentNode, _Owner, _Access) ->
+create_node_permission(Host, ServerHost, _Node, _ParentNode, Owner, Access) ->
-    %% TODO may we check bare JID match ?
+    LOwner = jlib:jid_tolower(Owner),
-    {result, true}.
+    {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

src/mod_pubsub/node_zoo.erl

@@ -91,16 +91,17 @@ 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 LOwner of
-    Allowed = case acl:match_rule(ServerHost, Access, LOwner) of
+	{"", Host, ""} ->
+	    true; % pubsub service always allowed
+	_ ->
+	    case acl:match_rule(ServerHost, Access, LOwner) of
 		allow ->
 		    true;
 		_ ->
-		    case Owner of
+		    false
-		    {jid, "", _, "", "", _, ""} -> true;
-		    _ -> false
 	    end
     end,
     {result, Allowed}.