Re: [PATCH 03/18] New observer behaviour

[Rob Taylor <rob taylor codethink co uk>]

Sergio Villar Senin wrote:
> Rob Taylor escribiu:
>> New observer behaviour. Observers will get folders_appeared events when
>> a tny_folder_store_refresh or tny_folder_store_get_folders occurs and
>> the cache is loaded for the first time. They get folders_created events
>> when a new folder appears that we didn't know about before. Observers
>> are notified about existing folders when they're attached.
> 
> Some comments
> 
> + if (folder && !g_hash_table_lookup_extended (priv->known_folders,
> folder, NULL, NULL)) {
> 
> No need to use the extended version.

See below.


> + if (folder && !g_hash_table_lookup_extended (priv->known_folders,
> folder, NULL, NULL)) {
> 
> Again not need to use _extended


actually its is important to use extended here and above. You can't
check for the *existance* of a key with the normal version.

> - if (!(iter->flags & CAMEL_FOLDER_VIRTUAL) &&
> _tny_folder_store_query_passes (query, iter) && priv->account)
> + if (!(iter->flags & CAMEL_FOLDER_VIRTUAL))
>
> Why we are not taking into account the query? And why removing the
> priv->account extra check?

If you read the patch carefully, you'll see that it's all wrapped in
if (iter && priv->account). The query check is done when prepending to
the returned list.

>  static void
>  tny_camel_folder_store_add_observer_default (TnyFolderStore *self,
> TnyFolderStoreObserver *observer)
>  {
>  	TnyCamelFolderPriv *priv = TNY_CAMEL_FOLDER_GET_PRIVATE (self);
> +	TnyFolderStoreChange *change = tny_folder_store_change_new (self);
> 
>  	g_assert (TNY_IS_FOLDER_STORE_OBSERVER (observer));
> 
> @@ -6100,6 +6206,10 @@ tny_camel_folder_store_add_observer_default
> (TnyFolderStore *self, TnyFolderStor
>  	}
>  	g_static_rec_mutex_unlock (priv->obs_lock);
> 
> +	g_hash_table_foreach (priv->known_folders, build_appeared_change, change);
> +	notify_folder_store_observers_about_in_idle (self, change,
> TNY_FOLDER_PRIV_GET_SESSION (priv));
> +	g_object_unref (change);
> +
> 
> I don't understand this part, why do we notify about appeared folders
> each time an observer is added ?


The idea here was to allow a model to be written that could just be an
observer and not need to call get_folders. with this a model can get all
 the information it needs just from _update.



> 
> +	if (iter) {
> +		TnyFolderStoreChange *change = NULL;
> 
> -			if (was_new && folder != NULL)
> -				_tny_camel_folder_set_folder_info (self, folder, iter);
> +		while (iter) {
> +			/* Also take a look at camel-maildir-store.c:525 */
> +			if (!(iter->flags & CAMEL_FOLDER_VIRTUAL)) {
> 
> Again in the store_account this time we're completely ignoring the query
> parameter.

No, the query is checked when prepending to the list.


Thanks,
Rob

> Br


-- 
Rob Taylor, Codethink Ltd. - http://codethink.co.uk