[Bug 24908] Communication policy (blocking policy) API

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=24908

Eitan Isaacson <eitan.isaacson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|review- but worth merging   |
                   |fairly soon                 |

--- Comment #24 from Eitan Isaacson <eitan.isaacson at collabora.co.uk> 2010-09-07 16:16:03 PDT ---
(In reply to comment #23)
> I like the "shape" of this interface now, but I'd prefer to not merge it as a
> draft until A_C_T has consecutive values, so that the first merged draft is
> something we could reasonably undraft.
> 
> > Access_Control_Type
> 
> The new values (Closed, Not_Understood and Subscribe_Or_Publish_List) are
> non-contiguous; they should get consecutive values.

64a1a14

> 
> Rather than talking about allowing communication or letting contacts see rich
> presence, I think this should be phrased in terms of "allow [x]", like you did
> for Whitelist.

a018e2e

> 
> If A_C[_T] is meant to replace R_P_A_C[_T], I'd actually prefer it to go
> straight in to the SimplePresence interface, with a note saying "new interfaces
> should use this one", and notes in R_P_A_C[_T] saying "Location uses this for
> historical reasons, new interfaces will use A_C[_T]" and "New values MUST NOT
> be added to this enum unless they are equal to the corresponding value of
> Access_Control_Type".
> 

64a1a14

> We should still have an Access_Control struct equivalent to R_P_A_C, even if
> CommunicationPolicy won't actually use it.
> 

Added it back, as you suggested below. I wish I remember why I removed it, I
think for simplicity. Future proofing and extendability is more important here.

0420594

> I think the addition of Closed (exactly equivalent to an empty whitelist) and
> Not_Understood is sufficiently non-controversial to put them straight in as
> stable API, tbh.

Great, they are in there.

> 
> If you insist on omitting the associated variant in the CommunicationPolicy
> interface, the descriptions of access control types should still talk about it:
> 
>     Whitelist = 0
> 
>     Only allow contacts that are in a certain whitelist.
> 
>     Setting this policy only makes sense in the context of an
>     Access_Control structure, in which the variant must contain
>     a list of _Contact_Handle_ representing the whitelist, with
>     signature 'au'.
> 

Policy will always be set with an associated variant, because of the API
change. So I changed the language.
53ef1ba

> Do we have a use-case for S_O_P_L (i.e. a form of communication in some
> protocol that can have this restriction), or should it be shelved for now? I
> believe the use-case was that it was a possible access control mode for Skype
> avatars, in which case we could add it if/when we add access control to the
> Avatars interface :-)

Yeah, we could add this in the future.

I think Conn.I.Avatars needs to be documented as a rich presence iface, I don't
think it is clear enough.

> 
> The API you've chosen for SetPolicy etc. excludes the possibility of ever using
> an access control type that needs a variant (e.g. Whitelist or Group) in
> CommunicationPolicy.
> 
> I'm not sure that that's a good idea - even if we don't need the variant for
> any of the access control types we support *now*, it doesn't cost us much to
> have a variant with a dummy value (i.e. replace the Access_Control_Type in
> SetPolicy, PolicyChanged and ActivePolicies with an Access_Control (uv), but
> keep Access_Control_Type in SupportedPolicies).
> 

Changed. 
0420594

> > +  <tp:copyright>Copyright (C) 2010 Collabora Limited</tp:copyright>
> 
> Nitpicking: can we have a © instead of ASCII art? :-)

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.