Bug 54780

Summary:	McpAccountStorage::altered-one basically only works for parameters
Product:	Telepathy	Reporter:	Simon McVittie <smcv>
Component:	mission-control	Assignee:	Simon McVittie <smcv>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	guillaume.desmottes, jonny.lamb, smcv, xclaesse
Version:	unspecified	Keywords:	patch
Hardware:	Other
OS:	All
URL:	http://cgit.freedesktop.org/~smcv/telepathy-mission-control/log/?h=altered-one-54780
Whiteboard:	one-way-migration
i915 platform:		i915 features:
Bug Depends on:	54873
Bug Blocks:	54874, 54875

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.