After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 724907 - GrlNetWc cache is not persistent
GrlNetWc cache is not persistent
Status: RESOLVED OBSOLETE
Product: grilo
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2014-02-21 18:50 UTC by Juan A. Suarez Romero
Modified: 2018-09-24 09:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
net: Avoid creating multiple caches (889 bytes, patch)
2014-03-28 17:10 UTC, Bastien Nocera
committed Details | Review
net: Add persistent cache (6.28 KB, patch)
2014-03-28 17:10 UTC, Bastien Nocera
needs-work Details | Review
lua-factory: Use persistent cache for grl.fetch() (919 bytes, patch)
2014-03-28 17:11 UTC, Bastien Nocera
none Details | Review

Description Juan A. Suarez Romero 2014-02-21 18:50:20 UTC
Currently the GrlNetWc cache is not persistent, so it is not re-used between sessions.

It would be great to have the choice of making it persistent.
Comment 1 Bastien Nocera 2014-02-21 18:58:12 UTC
I would add 2 new properties (and make the rest of the things that aren't properties, real properties...):
- cache-id
- persistent

And a grl_net_wc_get_for_id(), which would get a unique GrlNetWc for that ID (either a new object, or a reference to an existing object). Otherwise 2 GrlNetWc with the same ID would trample on each other's cache indexes.
Comment 2 Juan A. Suarez Romero 2014-02-21 23:42:36 UTC
We had a discussion in mailing list, https://mail.gnome.org/archives/grilo-list/2014-February/msg00016.html.

Not sure if you really need the persistent property. I think if it has a cache-id it is persistent, if it is NULL then it is not persistent.

Regarding the grl_net_wc_get_for_id(), it seems you suggests that the GrlNetWc is the one generating the id. I was thinking on the opposite: the source creates its own id. Reason is that the other way requires the source storing in some place the cache-id to recover it later. The source can have a hard-coded one, because each source would use only one GrlNetWc object, and thus a single cache.

As discussed in ml, I was thinking on storing the cache content in a different place based on the cache-id and also on the application using Grilo. Thus if you have two application running at the same time, like Totem and Gnome-music, even if they use the same source (which would be using the same hardcoded cache-id) both caches would be in different places.
Comment 3 Bastien Nocera 2014-03-28 13:34:29 UTC
(In reply to comment #2)
> We had a discussion in mailing list,
> https://mail.gnome.org/archives/grilo-list/2014-February/msg00016.html.
> 
> Not sure if you really need the persistent property. I think if it has a
> cache-id it is persistent, if it is NULL then it is not persistent.
> 
> Regarding the grl_net_wc_get_for_id(), it seems you suggests that the GrlNetWc
> is the one generating the id.

No. See the other "get_for" or "new_for" functions in GLib, GIO, etc. The ID is an argument that's passed in.
Comment 4 Bastien Nocera 2014-03-28 17:10:25 UTC
Created attachment 273184 [details] [review]
net: Avoid creating multiple caches

Call grl_net_wc_set_cache(wc, TRUE) 4 times, your SoupSession
will end up with 4 caches. Not what we want.
Comment 5 Bastien Nocera 2014-03-28 17:10:30 UTC
Created attachment 273185 [details] [review]
net: Add persistent cache

If a cache-id is passed when creating the GrlNetWc object, that
cache will be persistent across application runs.

This allows faster image displays, remembering thumbnails, etc.
Comment 6 Bastien Nocera 2014-03-28 17:11:33 UTC
Created attachment 273186 [details] [review]
lua-factory: Use persistent cache for grl.fetch()

Quicker Lua online sources!

The ID is the one defined in each of the lua scripts.
Comment 7 Bastien Nocera 2014-03-28 18:16:48 UTC
Review of attachment 273185 [details] [review]:

This isn't great.

We'll go with a "global" grl_net_wc_set_application_id(); which application will set if they want a persistent cache, and are single-instance (or they will come up with their own IDs).
We should also deprecate "cache-size", which would mean applications trampling on each other.

When grl_net_wc_set_application_id() is called, we'll set up a shared SoupCache for all the GrlNetWc instances to use (see bug 727261).
If grl_net_wc_set_application_id() is not called before the first GrlNetWc instance is created, then we won't be setting up a persistent cache, and revert to the old (current) behaviour.
Comment 8 Juan A. Suarez Romero 2014-03-28 23:04:14 UTC
Let's use grl_net_wc_set_cache_id() to explicitly said we are setting an ID for caching purpose.

The cache ID can be the same or a different ID than an application ID.
Comment 9 Juan A. Suarez Romero 2014-03-28 23:07:46 UTC
Review of attachment 273184 [details] [review]:

Looks good.
Comment 10 Bastien Nocera 2014-04-08 13:20:15 UTC
Comment on attachment 273184 [details] [review]
net: Avoid creating multiple caches

Attachment 273184 [details] pushed as 2c5e804 - net: Avoid creating multiple caches
Comment 11 GNOME Infrastructure Team 2018-09-24 09:25:54 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/grilo/issues/42.