Fix group flags for streamedmedia calls.

We start off with lots of flags, and then delete them as we work out what
kind of channel we are, rather than trying to track what we need to
add/remove over time. We should always have the right flags before we are
advertised on the bus.

Note that adding members to a channel made using Create isn't *strictly*
correct way to do it, but it isn't special-cased in any of the code, and
I don't want to set flags that are lies so I've just updated the tests
accordingly.

Also, converting the flags to their binary representation makes failed
assertions a lot easier to debug (and I honestly don't care about speed).

3 files changed, 33 insertions, 26 deletions

diff --git a/src/sip-media-channel.c b/src/sip-media-channel.c
index 41d949f..b78a57f 100644
--- a/src/sip-media-channel.c
+++ b/src/sip-media-channel.c
@@ -223,9 +223,13 @@ tpsip_media_channel_constructed (GObject *obj)
   tp_group_mixin_change_members (obj, "", add, NULL, NULL, NULL, 0, 0);
   tp_intset_destroy (add);
 
-  /* Allow member adding; also, we implement the 0.17.6 properties */
+  /* We start off with lots of flags, and then delete them as we work out what
+   * kind of channel we are, rather than trying to track what we need to
+   * add/remove over time. We should always have the right flags before we are
+   * advertised on the bus. */
   tp_group_mixin_change_flags (obj,
-      TP_CHANNEL_GROUP_FLAG_CAN_ADD | TP_CHANNEL_GROUP_FLAG_PROPERTIES, 0);
+      TP_CHANNEL_GROUP_FLAG_CAN_ADD | TP_CHANNEL_GROUP_FLAG_CAN_REMOVE |
+      TP_CHANNEL_GROUP_FLAG_CAN_RESCIND | TP_CHANNEL_GROUP_FLAG_PROPERTIES, 0);
 }
 
 static void tpsip_media_channel_dispose (GObject *object);
@@ -1472,9 +1476,8 @@ static void priv_session_state_changed_cb (TpsipMediaSession *session,
                                      self_handle,        /* actor */
                                      TP_CHANNEL_GROUP_CHANGE_REASON_INVITED);
 
-      /* update flags: allow removal and rescinding, no more adding */
-      tp_group_mixin_change_flags ((GObject *)channel,
-          TP_CHANNEL_GROUP_FLAG_CAN_REMOVE | TP_CHANNEL_GROUP_FLAG_CAN_RESCIND,
+      /* update flags: no more adding */
+      tp_group_mixin_change_flags ((GObject *)channel, 0,
           TP_CHANNEL_GROUP_FLAG_CAN_ADD);
 
       break;
@@ -1490,10 +1493,15 @@ static void priv_session_state_changed_cb (TpsipMediaSession *session,
                                      priv->initiator, /* actor */
                                      TP_CHANNEL_GROUP_CHANGE_REASON_INVITED);
 
-      /* No adding more members to the incoming call, removing is OK */
-      tp_group_mixin_change_flags ((GObject *) channel,
-          TP_CHANNEL_GROUP_FLAG_CAN_REMOVE,
-          TP_CHANNEL_GROUP_FLAG_CAN_ADD);
+      /* No adding more members to the incoming call. Therefore also not
+       * possible to add anyone to remote-pending, so rescinding would make
+       * utterly no sense. We also disallow removing the remote peer if
+       * we are not the initiator, so disallow that too.
+       * Removing yourself to end the call is not represented by group flags.
+       */
+      tp_group_mixin_change_flags ((GObject *) channel, 0,
+          TP_CHANNEL_GROUP_FLAG_CAN_ADD | TP_CHANNEL_GROUP_FLAG_CAN_REMOVE |
+          TP_CHANNEL_GROUP_FLAG_CAN_RESCIND);
 
       break;
 
@@ -1527,11 +1535,12 @@ static void priv_session_state_changed_cb (TpsipMediaSession *session,
                                          self_handle, 0);
         }
 
-      /* update flags: allow removal, deny adding and rescinding */
-      tp_group_mixin_change_flags ((GObject *)channel,
-          TP_CHANNEL_GROUP_FLAG_CAN_REMOVE,
-          TP_CHANNEL_GROUP_FLAG_CAN_ADD
-          | TP_CHANNEL_GROUP_FLAG_CAN_RESCIND);
+      /* update flags: deny adding and rescinding. Removing the remote peer is
+       * still allowed.
+       * Removing yourself to end the call is not represented by group flags.
+       */
+      tp_group_mixin_change_flags ((GObject *)channel, 0,
+          TP_CHANNEL_GROUP_FLAG_CAN_ADD | TP_CHANNEL_GROUP_FLAG_CAN_RESCIND);
 
       break;
 
@@ -1744,9 +1753,9 @@ _tpsip_media_channel_add_member (GObject *iface,
                                      TP_CHANNEL_GROUP_CHANGE_REASON_INVITED);
       tp_intset_destroy (remote_pending);
 
-      /* update flags: allow removal and rescinding, no more adding */
-      tp_group_mixin_change_flags (iface,
-          TP_CHANNEL_GROUP_FLAG_CAN_REMOVE | TP_CHANNEL_GROUP_FLAG_CAN_RESCIND,
+      /* update flags: no more adding.
+       * Removal and rescinding are still allowed. */
+      tp_group_mixin_change_flags (iface, 0,
           TP_CHANNEL_GROUP_FLAG_CAN_ADD);
 
       return TRUE;
diff --git a/tests/twisted/voip/incoming-basics.py b/tests/twisted/voip/incoming-basics.py
index 0bc7c2b..c3cf8dd 100644
--- a/tests/twisted/voip/incoming-basics.py
+++ b/tests/twisted/voip/incoming-basics.py
@@ -86,13 +86,12 @@ def test(q, bus, conn, sip_proxy, peer='foo@bar.com'):
 
     flags = group_props['GroupFlags']
     assert flags & cs.GF_PROPERTIES, flags
-    # FIXME #32189: sofiasip's group flags are borked.
     # Changing members in any way other than adding or removing yourself is
     # meaningless for incoming calls, and the flags need not be sent to change
     # your own membership.
-    #assert not flags & cs.GF_CAN_ADD, flags
-    #assert not flags & cs.GF_CAN_REMOVE, flags
-    #assert not flags & cs.GF_CAN_RESCIND, flags
+    assert not flags & cs.GF_CAN_ADD, flags
+    assert not flags & cs.GF_CAN_REMOVE, flags
+    assert not flags & cs.GF_CAN_RESCIND, flags
 
     assert group_props['Members'] == [remote_handle], group_props['Members']
     assert group_props['RemotePendingMembers'] == [], \
diff --git a/tests/twisted/voip/outgoing-basics.py b/tests/twisted/voip/outgoing-basics.py
index 2e90e92..933b4ad 100644
--- a/tests/twisted/voip/outgoing-basics.py
+++ b/tests/twisted/voip/outgoing-basics.py
@@ -147,15 +147,14 @@ def worker(q, bus, conn, sip_proxy, variant, peer):
             # but we should be allowed to add the peer.
             chan.Group.AddMembers([remote_handle], 'I love backwards compat')
 
-    base_flags = cs.GF_PROPERTIES | cs.GF_MESSAGE_REMOVE \
-               | cs.GF_MESSAGE_REJECT | cs.GF_MESSAGE_RESCIND
+    base_flags = cs.GF_PROPERTIES | cs.GF_CAN_REMOVE | cs.GF_CAN_RESCIND
 
-    if variant == REQUEST_ANONYMOUS_AND_ADD or variant == REQUEST_ANONYMOUS:
+    if variant in [REQUEST_ANONYMOUS_AND_ADD, REQUEST_ANONYMOUS, CREATE]:
         expected_flags = base_flags | cs.GF_CAN_ADD
     else:
         expected_flags = base_flags
-    # FIXME: 32189: group flags are borked.
-    #assertEquals(expected_flags, group_props['GroupFlags'])
+
+    assertEquals(bin(expected_flags), bin(group_props['GroupFlags']))
     assertEquals({}, group_props['HandleOwners'])
 
     assertEquals([], chan.StreamedMedia.ListStreams())