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 761852 - [lua] plugins don't fall through on empty resultset
[lua] plugins don't fall through on empty resultset
Status: RESOLVED OBSOLETE
Product: grilo
Classification: Other
Component: lua
git master
Other Linux
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2016-02-11 09:58 UTC by Marinus Schraal
Modified: 2018-09-24 09:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: check if core fallback to other lua-sources (10.12 KB, patch)
2016-03-15 22:00 UTC, Victor Toso
needs-work Details | Review

Description Marinus Schraal 2016-02-11 09:58:00 UTC
I noticed the lua lastfm-cover plugin doesn't fall through to eg. the spotify cover plugin if no cover images are found. Apparently an empty set is currently considered resolved in lua.

The patch adapts the plugin to truly return nothing, with some debug statements to see what's going on.

Example debug output of gnome-music trying to resolve cover images :

(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:496: fetch_done element 1 of 1 urls
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size small = 
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: V nil
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size medium = 
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: V nil
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size large = 
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: V nil
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size extralarge = 
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: V nil
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size mega = 
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: V nil
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size  = 
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: V nil
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: lastfm return empty
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1003: grl.callback()
(gnome-music:6100): Grilo-DEBUG: [source] grl-source.c:1980: resolve_result_relay_cb
(gnome-music:6100): Grilo-DEBUG: [source] grl-source.c:933: operation_set_finished (150)
(gnome-music:6100): Grilo-DEBUG: [wc] grl-net-wc.c:251: cache down
(gnome-music:6100): Grilo-DEBUG: [wc] grl-net-wc.c:251: cache down
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:496: fetch_done element 1 of 1 urls
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size small = http://img2-ak.lst.fm/i/u/34s/9e3d9a4bb5d3f9a0792558887afff578.png
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size medium = http://img2-ak.lst.fm/i/u/64s/9e3d9a4bb5d3f9a0792558887afff578.png
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size large = http://img2-ak.lst.fm/i/u/174s/9e3d9a4bb5d3f9a0792558887afff578.png
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size extralarge = http://img2-ak.lst.fm/i/u/300x300/9e3d9a4bb5d3f9a0792558887afff578.png
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size mega = http://img2-ak.lst.fm/i/u/9e3d9a4bb5d3f9a0792558887afff578.png
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1054: Image size  = http://img2-ak.lst.fm/i/u/arQ/9e3d9a4bb5d3f9a0792558887afff578.png
(gnome-music:6100): Grilo-DEBUG: [lua-library] grl-lua-library.c:1003: grl.callback()
(gnome-music:6100): Grilo-DEBUG: [source] grl-source.c:1980: resolve_result_relay_cb
(gnome-music:6100): Grilo-DEBUG: [source] grl-source.c:933: operation_set_finished (195)
Comment 1 Victor Toso 2016-02-22 13:42:08 UTC
Problem: after lua-factory tries to resolve in lastfm-cover it will not try another source in case of failure.

I would like to confirm with you if your patch from bug 761694 does not fix this by fixing lastfm-cover to return nothing instead of empty table.

Otherwise, this is possibly a bug in the core as it should fallback in different sources from the same plugin.
Comment 2 Marinus Schraal 2016-02-22 17:59:00 UTC
It positively does not fix this with the patch as provided in bug #761694 .

It is hard to discern which plugin is being resolved in the debug info, however if I remove the lastfm plugin I can see the spotify plugin making api calls. This does not ever happen if the lastfm plugin is available, even if covers are not being found.
Comment 3 Juan A. Suarez Romero 2016-03-04 16:47:16 UTC
(In reply to Victor Toso from comment #1)
> Problem: after lua-factory tries to resolve in lastfm-cover it will not try
> another source in case of failure.
> 
> I would like to confirm with you if your patch from bug 761694 does not fix
> this by fixing lastfm-cover to return nothing instead of empty table.
> 
> Otherwise, this is possibly a bug in the core as it should fallback in
> different sources from the same plugin.

Right: it's core task to use other sources if the former one couldn't resolve a key.

Could be lastfm-cover is adding an empty value? In this case, core assumes the key was resolved and doesn't try with other sources.
Comment 4 Bastien Nocera 2016-03-04 17:11:30 UTC
Can you provide a minimal test case for this problem? That would make it easier to reproduce the problem, certainly.
Comment 5 Victor Toso 2016-03-15 22:00:45 UTC
Created attachment 324051 [details] [review]
tests: check if core fallback to other lua-sources

In order to check if different lua-sources are called in case one fails,
the resolve_fallback tests inserts an specific url which only
basic-resolve.lua should be able to handle.
Comment 6 Victor Toso 2016-03-15 22:06:34 UTC
The above unit test is in top of Bug 763046

(In reply to Juan A. Suarez Romero from comment #3)
> Right: it's core task to use other sources if the former one couldn't
> resolve a key.

In my test, I have three fake lua-sources and all three of them were called but only the last one resolved the media. So I think core is fine!

> Could be lastfm-cover is adding an empty value? In this case, core assumes
> the key was resolved and doesn't try with other sources.

That was the case but as per comment #1, the following patch fixed the wrong return in lastfm-cover...
https://git.gnome.org/browse/grilo-plugins/commit/?id=15023e0137e5ed99ecc2

Maybe lua-library is doing something wrong that the unit test did not cover..
Comment 7 Bastien Nocera 2016-03-17 15:01:41 UTC
Review of attachment 324051 [details] [review]:

How do you make sure that basic-resolve.lua is called after lua-errors.lua?

Apart from that, and if the test passes, looks fine to me.
Comment 8 GNOME Infrastructure Team 2018-09-24 09:41:07 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/79.