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 758654 - Fix tracker serialisation
Fix tracker serialisation
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: plugins
unspecified
Other All
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2015-11-25 13:12 UTC by Bastien Nocera
Modified: 2015-12-16 09:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tracker: Add IDs to top-level tracker boxes (2.15 KB, patch)
2015-11-25 13:12 UTC, Bastien Nocera
committed Details | Review
tracker: Map container ID to tracker categories (1.98 KB, patch)
2015-11-25 13:12 UTC, Bastien Nocera
none Details | Review
source: Warn against broken media boxes (1.57 KB, patch)
2015-11-25 13:19 UTC, Bastien Nocera
committed Details | Review
core: Remove trailing '/' from GrlMedia IDs (1.37 KB, patch)
2015-12-01 15:19 UTC, Bastien Nocera
committed Details | Review
tracker: Map container ID to tracker categories (2.74 KB, patch)
2015-12-01 15:23 UTC, Bastien Nocera
committed Details | Review
tracker: ensure ID is always obtained (1.66 KB, patch)
2015-12-14 08:42 UTC, Juan A. Suarez Romero
committed Details | Review
doc: update documentation (22.96 KB, patch)
2015-12-16 09:41 UTC, Juan A. Suarez Romero
rejected Details | Review

Description Bastien Nocera 2015-11-25 13:12:30 UTC
.
Comment 1 Bastien Nocera 2015-11-25 13:12:35 UTC
Created attachment 316234 [details] [review]
tracker: Add IDs to top-level tracker boxes
Comment 2 Bastien Nocera 2015-11-25 13:12:41 UTC
Created attachment 316235 [details] [review]
tracker: Map container ID to tracker categories

Combined with the previous commit, this allows:
grl-launch-0.3 -S browse -k title grlbox://grl-tracker-source/videos
to actually work. Otherwise the category would be none, and we'd always
print the top-level categories.
Comment 3 Bastien Nocera 2015-11-25 13:19:46 UTC
Created attachment 316238 [details] [review]
source: Warn against broken media boxes

All media boxes should have IDs for us to be able to serialise, or
deserialise it, and, for example, browse the sub-boxes in some sources
with grl-launch.
Comment 4 Victor Toso 2015-11-27 09:49:09 UTC
Review of attachment 316235 [details] [review]:

::: src/tracker/grl-tracker-source-api.c
@@ +1059,3 @@
+      category = "nmm:Video";
+    else {
+      /* Error out */

No need to set GError?

GRL_WARNING to help debugging
Comment 5 Victor Toso 2015-11-27 09:50:00 UTC
Review of attachment 316234 [details] [review]:

looks good
Comment 6 Victor Toso 2015-11-27 09:53:44 UTC
Review of attachment 316238 [details] [review]:

Looks good
Comment 7 Bastien Nocera 2015-12-01 12:29:52 UTC
(In reply to Victor Toso from comment #4)
> Review of attachment 316235 [details] [review] [review]:
> 
> ::: src/tracker/grl-tracker-source-api.c
> @@ +1059,3 @@
> +      category = "nmm:Video";
> +    else {
> +      /* Error out */
> 
> No need to set GError?
> 
> GRL_WARNING to help debugging

Probably should, yes.

But that patch also breaks navigation at the top-level of the source when using grilo-test-ui.
Comment 8 Bastien Nocera 2015-12-01 13:38:56 UTC
Review of attachment 316235 [details] [review]:

This breaks when browsing in grilo-test-ui because it *does* pass a container with a NULL ID, which is what we're trying to detect. Complicated...
Comment 9 Bastien Nocera 2015-12-01 13:40:11 UTC
3765   if (!container) {•
3766     /* Special case: NULL container ==> NULL id */•
3767     bs->container = grl_media_box_new ();•
3768     grl_media_set_source (bs->container,•
3769                           grl_source_get_id (source));•

Sad trombon.
Comment 10 Bastien Nocera 2015-12-01 15:19:10 UTC
Created attachment 316609 [details] [review]
core: Remove trailing '/' from GrlMedia IDs

grl-launch-0.3 -S browse -k title grlbox://grl-tracker-source/videos/
should have the same output as:
grl-launch-0.3 -S browse -k title grlbox://grl-tracker-source/videos

The first would get its ID parsed to "videos/" whereas the second would
be parsed to "videos". Chomp the trailing '/'.
Comment 11 Bastien Nocera 2015-12-01 15:23:17 UTC
Created attachment 316611 [details] [review]
tracker: Map container ID to tracker categories

Combined with the previous commit, this allows:
grl-launch-0.3 -S browse -k title grlbox://grl-tracker-source/videos
to actually work. Otherwise the category would be none, and we'd always
print the top-level categories.
Comment 12 Juan A. Suarez Romero 2015-12-14 07:43:32 UTC
Review of attachment 316609 [details] [review]:

LGTM
Comment 13 Juan A. Suarez Romero 2015-12-14 08:42:07 UTC
Created attachment 317335 [details] [review]
tracker: ensure ID is always obtained

GRL_METADATA_KEY_ID is almost a mandatory key, and thus must be obtained
even if user doesn't request it.
Comment 14 Juan A. Suarez Romero 2015-12-14 08:42:25 UTC
Review of attachment 316611 [details] [review]:

+1
Comment 15 Bastien Nocera 2015-12-14 10:40:59 UTC
Attachment 316238 [details] pushed as 0b7ed08 - source: Warn against broken media boxes
Attachment 316609 [details] pushed as 6d68858 - core: Remove trailing '/' from GrlMedia IDs
Comment 16 Bastien Nocera 2015-12-14 10:43:22 UTC
Attachment 316234 [details] pushed as 5a2cf05 - tracker: Add IDs to top-level tracker boxes
Attachment 316611 [details] pushed as eaef66e - tracker: Map container ID to tracker categories
Attachment 317335 [details] pushed as c74848f - tracker: ensure ID is always obtained
Comment 17 Juan A. Suarez Romero 2015-12-16 09:41:52 UTC
Created attachment 317486 [details] [review]
doc: update documentation

No more GrlMediaFoos.

https://bugzilla.gnome.org/show_bug.cgi?id=755551
Comment 18 Juan A. Suarez Romero 2015-12-16 09:43:03 UTC
Review of attachment 317486 [details] [review]:

Wrong patch added.