GNOME Bugzilla – Bug 774748
tracker: Support favorite read/write
Last modified: 2016-12-02 21:24:44 UTC
This is a proposed solution for reading/writing the GRL_METADATA_KEY_FAVOURITE in the tracker plugin. The issue is that trackers boolean representation does not fit with tracker's creating/deleting of a tag (https://developer.gnome.org/ontology/stable/nao-Tag.html). More of trackers properties do not line up with grilo. So if at some point we might want to handle more of those keys, it might require coming up with a more generic solution.
Created attachment 340354 [details] [review] tracker: Support favorite read/write Support metadata KEY_FAVOURITE reading & writing in the tracker backend. The tracker ontology for this uses hasTag in combination with the nao:predefined-tag-favorite property, which doesn't line up directly with the boolean format Grilo uses. So either set the key with the given property or delete the tag completely.
Created attachment 340358 [details] [review] tracker: Support favorite read/write Support metadata KEY_FAVOURITE reading & writing in the tracker backend. The tracker ontology for this uses hasTag in combination with the nao:predefined-tag-favorite property, which doesn't line up directly with the boolean format Grilo uses. So either set the key with the given property or delete the tag completely.
Review of attachment 340358 [details] [review]: Other than that, looks okay too me. I'll post a patch here reorganing grl_tracker_tracker_get_insert_string(). Feel free to rebase in top of it or just rework your patch and I'll rebase in top of it before pushing ::: src/tracker/grl-tracker-utils.c @@ +96,3 @@ + gint column, + GrlMedia *media, + GrlKeyID key) indentation @@ +98,3 @@ + GrlKeyID key) +{ + const gchar *str = tracker_sparql_cursor_get_string (cursor, column, NULL); as str could be NULL, you will need to check that before g_str_has_suffix().. what about: { const gchar *str = tracker_sparql_cursor_get_string (cursor, column, NULL); gboolean is_favourite = FALSE; if (str != NULL && g_str_has_suffix (str, "predefined-tag-favorite")) is_favourite = TRUE; grl_media_set_favourite (media, is_favourite); } @@ +508,3 @@ + g_string_append_printf (gstr, "%s nao:predefined-tag-favorite", + assoc->sparql_key_attr); + } ident @@ +532,3 @@ if (grl_data_has_key (GRL_DATA (media), GRLPOINTER_TO_KEYID (key->data))) { + /* The favourite key is really setting or deleteing a tag typo: deleting @@ +535,3 @@ + * in tracker, so in the case of setting it to false skip + * the insert string creation step for this key completely. + */ Based on the comment, we don't need to check the metadadata-key value of favourite in the if, right?
Created attachment 340466 [details] [review] tracker: small rework on _tracker_get_insert_string() Clear a bit the logic so it gets easier to include changes. This patch does: - Change GList iteration from while() to for() - Move variables to its local scope; - Use *continue* inside the for; - Remove if else check on gboolean first
Created attachment 340507 [details] [review] tracker: Support favorite read/write Support GRL_METADATA_KEY_FAVOURITE reading & writing in the tracker backend. The tracker ontology for this uses hasTag in combination with the nao:predefined-tag-favorite property, which doesn't line up directly with the boolean format Grilo uses. So either set the key with the given property or delete the tag completely.
(In reply to Victor Toso from comment #3) > Review of attachment 340358 [details] [review] [review]: > Based on the comment, we don't need to check the metadadata-key value of > favourite in the if, right? It does need to be checked. We want to skip creating an insert string when the key value is set to false: we want the tag to be removed completely. Updating values is implemented in tracker by deleting the value completely and then inserting it anew (this how all changes work). Not providing an insert string thus deletes it. Note that this TRACKER_DELETE_REQUEST only applies to the case in which we manipulate only 1 metadata key (like i want to do in music) and just this one because it skips the insert string creation. In the case of multiple keys it will always use TRACKER_SAVE_REQUEST, but with the favourite string skipped and other properties treated as regular this amounts to the same outcome: the favourite tag to be removed and the others to be updated.
Created attachment 340561 [details] [review] tracker: Support last-played metadata storage Allows storing the last played date-time in tracker. Use the nfo:fileLastAccessed & nie:contentAccessed property for this as they are both valid.
Created attachment 341238 [details] [review] tracker: Support last-played metadata storage Allows storing the last played date-time in tracker. Use the nfo:fileLastAccessed & nie:contentAccessed property for this as they are both valid.
Review of attachment 340507 [details] [review]: Ack! I'm fixing two small things and pushing. ::: src/tracker/grl-tracker-utils.c @@ +96,3 @@ + gint column, + GrlMedia *media, + GrlKeyID key) All the other functions indent the parameters vertically, just after the open bracket. @@ +541,3 @@ + if (assoc->grl_key == GRL_METADATA_KEY_FAVOURITE && + !grl_data_get_boolean (GRL_DATA(media), assoc->grl_key)) + continue; changing grl_data_get_boolean() to grl_media_get_favourite()
Attachment 340466 [details] pushed as 9183260 - tracker: small rework on _tracker_get_insert_string() Attachment 340507 [details] pushed as bb3a85d - tracker: Support favorite read/write Attachment 341238 [details] pushed as b07bf05 - tracker: Support last-played metadata storage