Fix set-subscriptions to work with multi-subscribe (EJAB-977)

SVN Revision: 2364

13 files changed, 57 insertions, 50 deletions

diff --git a/src/mod_pubsub/gen_pubsub_node.erl b/src/mod_pubsub/gen_pubsub_node.erl
index ba0d7386a..dbf7e25d5 100644
--- a/src/mod_pubsub/gen_pubsub_node.erl
+++ b/src/mod_pubsub/gen_pubsub_node.erl
@@ -58,7 +58,7 @@ behaviour_info(callbacks) ->
      {get_node_subscriptions, 1},
      {get_entity_subscriptions, 2},
      {get_subscriptions, 2},
-     {set_subscriptions, 3},
+     {set_subscriptions, 4},
      {get_states, 1},
      {get_state, 2},
      {set_state, 1},
diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl
index 6ddfc945d..a1ec03e62 100644
--- a/src/mod_pubsub/mod_pubsub.erl
+++ b/src/mod_pubsub/mod_pubsub.erl
@@ -2522,12 +2522,13 @@ set_subscriptions(Host, Node, From, EntitiesEls) ->
 					  xml:get_attr_s("jid", Attrs)),
 				  Subscription = string_to_subscription(
 						   xml:get_attr_s("subscription", Attrs)),
+                                  SubId = xml:get_attr_s("subid", Attrs),
 				  if
 				      (JID == error) or
 				      (Subscription == false) ->
 					  error;
 				      true ->
-					  [{jlib:jid_tolower(JID), Subscription} | Acc]
+					  [{jlib:jid_tolower(JID), Subscription, SubId} | Acc]
 				  end
 			  end
 		  end
@@ -2539,10 +2540,10 @@ set_subscriptions(Host, Node, From, EntitiesEls) ->
 	    Action = fun(#pubsub_node{owners = Owners, type = Type, id = NodeId}) ->
 			     case lists:member(Owner, Owners) of
 				 true ->
-				     lists:foreach(fun({JID, Subscription}) ->
-							   node_call(Type, set_subscriptions, [NodeId, JID, Subscription])
-						   end, Entities),
-				     {result, []};
+                                     lists:foreach(fun({JID, Subscription, SubId}) ->
+                                                           node_call(Type, set_subscriptions, [NodeId, JID, Subscription, SubId])
+                                                   end, Entities),
+                                     {result, []};
 				 _ ->
 				     {error, ?ERR_FORBIDDEN}
 			     end
@@ -2553,7 +2554,6 @@ set_subscriptions(Host, Node, From, EntitiesEls) ->
 	    end
     end.
 
-
 %% @spec (OwnerUser, OwnerServer, {SubscriberUser, SubscriberServer, SubscriberResource}, AllowedGroups)
 %%    -> {PresenceSubscription, RosterGroup}
 get_roster_info(OwnerUser, OwnerServer, {SubscriberUser, SubscriberServer, _}, AllowedGroups) ->
diff --git a/src/mod_pubsub/node.template b/src/mod_pubsub/node.template
index 534c18226..08f2e08ed 100644
--- a/src/mod_pubsub/node.template
+++ b/src/mod_pubsub/node.template
@@ -57,7 +57,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -157,8 +157,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_hometree:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscriptions) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscriptions).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).
diff --git a/src/mod_pubsub/node_buddy.erl b/src/mod_pubsub/node_buddy.erl
index 2e8dd0249..b3aea6f4a 100644
--- a/src/mod_pubsub/node_buddy.erl
+++ b/src/mod_pubsub/node_buddy.erl
@@ -58,7 +58,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -159,8 +159,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_hometree:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscriptions) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscriptions).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).
diff --git a/src/mod_pubsub/node_club.erl b/src/mod_pubsub/node_club.erl
index 91ff520c6..e3c16c630 100644
--- a/src/mod_pubsub/node_club.erl
+++ b/src/mod_pubsub/node_club.erl
@@ -58,7 +58,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -158,8 +158,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_hometree:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscription) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscription).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).
diff --git a/src/mod_pubsub/node_dag.erl b/src/mod_pubsub/node_dag.erl
index a276f08f0..2ffb70df2 100644
--- a/src/mod_pubsub/node_dag.erl
+++ b/src/mod_pubsub/node_dag.erl
@@ -42,7 +42,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -136,8 +136,8 @@ get_node_subscriptions(NodeID) ->
 get_subscriptions(NodeID, Owner) ->
     node_hometree:get_subscriptions(NodeID, Owner).
 
-set_subscriptions(NodeID, Owner, Subscriptions) ->
-    node_hometree:set_subscriptions(NodeID, Owner, Subscriptions).
+set_subscriptions(NodeID, Owner, Subscription, SubID) ->
+    node_hometree:set_subscriptions(NodeID, Owner, Subscription, SubID).
 
 get_states(NodeID) ->
     node_hometree:get_states(NodeID).
diff --git a/src/mod_pubsub/node_dispatch.erl b/src/mod_pubsub/node_dispatch.erl
index f67ab81fa..698d2559a 100644
--- a/src/mod_pubsub/node_dispatch.erl
+++ b/src/mod_pubsub/node_dispatch.erl
@@ -56,7 +56,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -162,8 +162,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(_NodeId, _Owner) ->
     {result, []}.
 
-set_subscriptions(NodeId, Owner, Subscription) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscription).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).
diff --git a/src/mod_pubsub/node_flat.erl b/src/mod_pubsub/node_flat.erl
index a4fbb3b67..4a89abdc6 100644
--- a/src/mod_pubsub/node_flat.erl
+++ b/src/mod_pubsub/node_flat.erl
@@ -49,7 +49,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -145,8 +145,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_hometree:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscription) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscription).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).
diff --git a/src/mod_pubsub/node_hometree.erl b/src/mod_pubsub/node_hometree.erl
index 764b511d4..cdad0699f 100644
--- a/src/mod_pubsub/node_hometree.erl
+++ b/src/mod_pubsub/node_hometree.erl
@@ -65,7 +65,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -691,23 +691,30 @@ get_subscriptions(NodeId, Owner) ->
     SubState = get_state(NodeId, SubKey),
     {result, SubState#pubsub_state.subscriptions}.
 
-set_subscriptions(NodeId, Owner, Subscriptions) ->
+set_subscriptions(NodeId, Owner, none, SubId) ->
     SubKey = jlib:jid_tolower(Owner),
     SubState = get_state(NodeId, SubKey),
+    case {SubId, SubState#pubsub_state.subscriptions} of
+        {_, []} ->
+            {error, ?ERR_ITEM_NOT_FOUND};
+        {"", [{_, SID}]} ->
+            unsub_with_subid(NodeId, SID, SubState);
+        {"", [_|_]} ->
+            {error, ?ERR_EXTENDED(?ERR_BAD_REQUEST, "subid-required")};
+        _ ->
+            unsub_with_subid(NodeId, SubId, SubState)
+    end.
 
-    OSIDs = [SID || {_, SID} <- SubState#pubsub_state.subscriptions],
-    NSIDs = [SID || {_, SID} <- Subscriptions],
-    RSIDs = OSIDs -- NSIDs,
-
-    lists:foreach(fun(SubID) ->
-			  pubsub_subscription:unsubscribe_node(SubKey, NodeId,
-							       SubID)
-		  end, RSIDs),
-    case {Subscriptions, SubState#pubsub_state.affiliation} of
+unsub_with_subid(NodeId, SubId, SubState) ->
+    pubsub_subscription:unsubscribe_node(SubState#pubsub_state.stateid,
+                                         NodeId, SubId),
+    NewSubs = lists:filter(fun ({_, SID}) -> SubId =/= SID end,
+                           SubState#pubsub_state.subscriptions),
+    case {NewSubs, SubState#pubsub_state.affiliation} of
 	{[], none} ->
-	    del_state(NodeId, SubKey);
+	    del_state(NodeId, element(1, SubState#pubsub_state.stateid));
 	_ ->
-	    set_state(SubState#pubsub_state{subscriptions = Subscriptions})
+	    set_state(SubState#pubsub_state{subscriptions = NewSubs})
     end.
 
 %% @spec (NodeId) -> [States] | []
diff --git a/src/mod_pubsub/node_mb.erl b/src/mod_pubsub/node_mb.erl
index 70ceabbb1..b7ac76238 100644
--- a/src/mod_pubsub/node_mb.erl
+++ b/src/mod_pubsub/node_mb.erl
@@ -61,7 +61,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -166,8 +166,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_pep:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscription) ->
-    node_pep:set_subscriptions(NodeId, Owner, Subscription).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_pep:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_pep:get_states(NodeId).
diff --git a/src/mod_pubsub/node_pep.erl b/src/mod_pubsub/node_pep.erl
index 8e2e7e0a3..57e30fecb 100644
--- a/src/mod_pubsub/node_pep.erl
+++ b/src/mod_pubsub/node_pep.erl
@@ -54,7 +54,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -233,8 +233,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_hometree:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscription) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscription).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).
diff --git a/src/mod_pubsub/node_private.erl b/src/mod_pubsub/node_private.erl
index e9c925403..f4d4c8dea 100644
--- a/src/mod_pubsub/node_private.erl
+++ b/src/mod_pubsub/node_private.erl
@@ -58,7 +58,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -162,8 +162,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_hometree:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscription) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscription).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).
diff --git a/src/mod_pubsub/node_public.erl b/src/mod_pubsub/node_public.erl
index ae7beff84..fa806b712 100644
--- a/src/mod_pubsub/node_public.erl
+++ b/src/mod_pubsub/node_public.erl
@@ -58,7 +58,7 @@
 	 get_entity_subscriptions/2,
 	 get_node_subscriptions/1,
 	 get_subscriptions/2,
-	 set_subscriptions/3,
+	 set_subscriptions/4,
 	 get_states/1,
 	 get_state/2,
 	 set_state/1,
@@ -158,8 +158,8 @@ get_node_subscriptions(NodeId) ->
 get_subscriptions(NodeId, Owner) ->
     node_hometree:get_subscriptions(NodeId, Owner).
 
-set_subscriptions(NodeId, Owner, Subscription) ->
-    node_hometree:set_subscriptions(NodeId, Owner, Subscription).
+set_subscriptions(NodeId, Owner, Subscription, SubId) ->
+    node_hometree:set_subscriptions(NodeId, Owner, Subscription, SubId).
 
 get_states(NodeId) ->
     node_hometree:get_states(NodeId).