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

SVN Revision: 1471

diff --git a/ChangeLog b/ChangeLog
index 23449e7686e019467823434f89ccb04243224d27..ef918d3596d0671bd149fec3f02c1265c4b1c93a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+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
+
 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 d9173ed83a1717b7fa3c5ea77347937c7a6cf0cb..3840f9f7878a3418a7fafa6b79f18c295f7c7faf 100644 (file)
--- 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, ServedHosts) ->
-    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),
     lists:foreach(
       fun(H) ->
@@ -419,7 +417,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:
@@ -516,9 +514,26 @@ 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),
+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) ->
@@ -1296,12 +1311,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()} |
@@ -1726,9 +1735,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(
@@ -2405,9 +2414,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.
@@ -2452,7 +2463,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 7b0d85996d130a05d83030e1e97b63d17b6d3472..afafd7e44f6bcdeb2a8a4ec538bfd120abcf1fe4 100644 (file)
--- a/src/mod_pubsub/node_default.erl
+++ b/src/mod_pubsub/node_default.erl
@@ -190,21 +190,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 13321b0e5a805314b8ecb42ee1d2c6143a8d1c13..8320ab9363fdb08e4c87d05073eab45043431185 100644 (file)
--- a/src/mod_pubsub/node_pep.erl
+++ b/src/mod_pubsub/node_pep.erl
@@ -109,9 +109,24 @@ 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