Bug 54780 - McpAccountStorage::altered-one basically only works for parameters
Summary: McpAccountStorage::altered-one basically only works for parameters
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://cgit.freedesktop.org/~smcv/tel...
Whiteboard: one-way-migration
Keywords: patch
Depends on: 54873
Blocks: 54874 54875
  Show dependency treegraph
 
Reported: 2012-09-11 15:09 UTC by Simon McVittie
Modified: 2013-02-13 14:45 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2012-09-11 15:09:24 UTC
Based on my testing, when McpAccountStorage::altered-one is emitted, it doesn't actually work for most account properties.

This is because mcd_account_property_changed() (renamed to mcd_account_altered_by_plugin() in one of my branches) tries to copy things from internal storage to D-Bus by calling getprop() followed by setprop(). As far as I can tell, the intention is that this retrieves the new value from McdStorage (where it was previously left by mcp_account_manager_set_value()), then saves it again. Unfortunately, most getprop() implementations work from the McdAccount's internal data structure, so they never see the new value...
Comment 1 Simon McVittie 2012-09-13 15:15:25 UTC
This branch follows on from the test for this feature added at the end of Bug #54873. It makes everything in that test actually work...

This is part 4 of my account storage refactoring megabranch.
Comment 2 Simon McVittie 2012-09-13 15:52:11 UTC
This branch contains a data migration step: AutomaticPresenceType, ...Status and ...Message are combined into a new (uss)-valued attribute, AutomaticPresence (yes, we now have a (really dumb) serialization for (uss) in keyfiles). It will not be safe to downgrade past the first release with this branch merged.
Comment 3 Will Thompson 2012-11-07 17:30:50 UTC
In commit 05d1af087:

@@ -514,10 +520,17 @@ mcp_account_storage_priority (const McpAccountStorage *storage)
  *  like "DisplayName", or "param-" plus a parameter like "account"
  *
  * Get a value from the plugin's in-memory cache.
- * The plugin is expected to call mcp_account_manager_set_value(),
- * and if appropriate, mcp_account_manager_parameter_make_secret(),
+ * The plugin is expected to call
+ * Before emitting this signal, the plugin must call
+ * either mcp_account_manager_set_attribute(),
+ * mcp_account_manager_set_parameter(),
+ * or mcp_account_manager_set_value() and (if appropriate)
+ * mcp_account_manager_parameter_make_secret()
  * before returning from this method call.

There's a stray “The plugin is expected to call” in there.

It otherwise looks fine.
Comment 4 Simon McVittie 2013-02-13 14:45:11 UTC
Fixed in git for 5.15.0


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.