[Bug 29672] Should implement profile support

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Andre Moreira Magalhaes <andrunko at gmail.com> 2010-08-19 21:34:07 PDT ---
(In reply to comment #2)
> I think we should have another constructor Profile() which would be used in the
> case setFileName() will be called anyway to prevent searching for / parsing the
> wrong file completely. Then we should have createForServiceName() and
> createForFilename() wrapping it, instead of the current create(serviceName) so
> that it's clearer what will actually be done. I think it's sensible anyway to
> have the filename version be public too anyway.
Implemented this but a little different than suggested. Profile now has only
one constructor that takes no parameter and private methods setServiceName and
setFileName that resets the profile data and re-parse it.
We can't have constructors taking protocol name and service name, as both are
QString...

Also renamed create to createForServiceName and added a new public method
createForFileName as suggested.

> You should either comment out the account profile stuff OR implement the fake
> profile, otherwise it's terribly confusing as mostly anybody using tp-qt4 on
> desktop will find that their account doesn't have a profile in fact.
>  - Agreed on IRC that fake profiles should be implemented, and that Account
> profiles should be Account::FeatureProfile, which depends on
> Account::FeatureProtocolInfo. This is needed to get the default parameters for
> the protocol to specify as optional service-specific parameters in the fake
> profile, but additionally allows us to potentially verify that the Protocol
> parameters / RCCs / etc as reported by the CM or its .manager file are actually
> compatible with info in an actual .profile file.
Done.

> In startElement, I don't think you should error out on unrecognized elements.
> If we happen to add new elements in a .profile file format update, we don't
> want old tp-qt4 versions to ignore them. Instead, in case you don't recognize
> the element, just leave out the check for not having attrs and being a child of
> the service node. Emit a warning about ignoring the element and continue
> parsing normally.
I think this can be done by changing xmlns with a new profile definition.
Changed anyway, it only warns now.

> The API leaves answering the question "for this set of fixedProperties in
> unsupportedChannelClasses, which capabilities are actually still available"
> quite hard, so UCC is quite useless :(. Do you think it would be sensible in
> the filter-by-rcc branch to add to the Account profile support something, that
> when there is no Connected Connection and the ProtocolInfo is used, would
> auto-subtract the profile UCC from the ProtocolInfo RCC so that the Account
> Capabilities would then report supportTextChats() etc correctly? Note that when
> there is a Connected Connection, it will anyway not report the RCCs the service
> doesn't support - the CM is expected to do the subtraction.
I think Profile::unsupportedChannelClasses should not be used directly by UIs,
instead they should use Account::capabilities that will be a merge of CM caps
and profile caps (or just the connection caps if online). The branch
filter-by-rcc adds a method capabilities() that should do the trick when
creating a ConnectionCapabilities object.

I can't see any better option here, we could add a ConnectionCapabilities
*unsupportedCapabilities() method instead, but it will be confusing anyway.

> The other option is to provide some kind of CapabilitiesBase proxy which takes
> a CapabilitiesBase and the unsupportedChannelClasses, and then acts based on
> the supplied CapabilitiesBase->requestableChannelClasses() and the
> unsupportedChannelClasses. Profile could have a function to return such a caps
> proxy instance. This seems a bit hairy though...
>
> Do you see some other straightforward option?
As said above I think this is work for Account::capabilities added in
filter-by-rcc. Now whether to return a RCCList or a ConnectionCapabilities * in
Profile::unsupportedCC/Caps I don't know.

> If this capabilities badgering is too hard to do now, let's leave it for bug
> #29484 but in this case let's leave a big fat TODO. We need to do something
> about RCCs in Profiles though to support eg. account creation UIs wanting to
> show "this profile would support voice calls, choose it!" to the user creating
> an account - there's no Account at that point, just the Profile exposing bare
> RCCs.
Once we decide about this I can update the bug. IMHO we should just leave as is
and make Account::capabilities do the work for us.

> allowOthersPresences should be allowOtherPresences.
Done :)

> Otherwise ++, I think. Please drop a comment on this bug when these are
> settled!
Thanks for the review, pushed a new version with the fixes stated and also
added more tests for Account::profile and docs.

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