I am affected by this bug. After renaming a folder, Music shows duplicates of all tracks within that folder (which happens to be the majority of my library) -- one at the old path, one at the new.

Is there a way for me to manually clear the database to remove the duplicate entries?

I would Like to Work on this Bug. I am a newbie here so it would be great if someone could guide me.

I'm attaching a stopgap patch to grilo-plugins that should "fix" the feature itself, however I think there's quite some room for improvement:

- On one hand, I started on a libtracker-notify helper library that abstracts usage of GraphUpdated. As It is thought out, should be suitable to replace grl-tracker-source-notif.c internals without external changes.

- IMO it's highly counterintuitive that the nfo:FileDataObject check I'm removing in the patch doesn't work, this should be fixed in the code collecting the changes for GraphUpdated emission in tracker-store. 

- gnome-music should also be listening to updates, there could be metadata changes, or files could be moved between folders inspected by Tracker, these would come up as just updates.

- gnome-music could in general be better at reacting to grilo changes:

1) In general, rebuilding all views might not be the best performance wise
2) For added media, there are no nie:url checks, this means gnome-music reacts to media being added in other folders than ~/Music.
3) for removed media, it is even worse... any remove event results in the views being updated (presumably because at the time of emission the media has unknown type and url [1]). This results in gnome-music reacting to eg. deleting a document in ~/Downloads. For the case of deletes, I'd advise propagating the tracker IDs further, and iterating across the different views removing the matches.

[1] Grilo could detect the type, kind of. The first string argument in the GraphUpdated event is the class notifying about the updates, nmm:MusicPiece in this case (with the expanded nmm: prefix)

Created attachment 337394 [details] [review]
tracker: Do not check for specific nfo:FileDataObject rdf:type changes

It is unreliable to rely on these, as this rdf:type may be deleted after
the one with the tracker:notify property, meaning that this resource isn't
eligible for notification through GraphUpdated anymore.

Review of attachment 337394 [details] [review]:

Looks good, thanks for the patch and sorry for taking time to review it.
Let me know if I can help with something when libtracker-notify is ready.

(In reply to Victor Toso from comment #5)
> Review of attachment 337394 [details] [review] [review]:
> 
> Looks good, thanks for the patch and sorry for taking time to review it.
> Let me know if I can help with something when libtracker-notify is ready.

Cheers :). I'm pushing, but will leave the bug open for the TrackerNotifier work. I'm looking into merging it soon in Tracker, and grilo is my first specimen in mind :).

Comment on attachment 337394 [details] [review]
tracker: Do not check for specific nfo:FileDataObject rdf:type changes

Attachment 337394 [details] pushed as 73c1aa1 - tracker: Do not check for specific nfo:FileDataObject rdf:type changes

Created attachment 341147 [details] [review]
view: Destroy album widgets before repopulating

Created attachment 341148 [details] [review]
grilo: Simplify source notification

Now that grilo behaves better wrt notifications in the tracker
plugin, this code can be made simpler, by just relying on 1) the
correctness of the Grl.Media data, and 2) that grilo will group
events to reduce notification overhead.

Created attachment 341149 [details] [review]
views: Set self._init on populate() for all views

Albums and artists views were missing this, making content change
notifications from grilo unheard.

Created attachment 341150 [details] [review]
views: Avoid some idle_add() calls

These aren't that necessary, since all queries are done async now.

The main patch might be btw considered as kinda wip. There's some areas of improvement:
- The notification handling collects the changed IDs, but those nothing with that info, everything is just queried again. This might be more or less ok, but
- The handling of updates misses some worthwhile things, mainly so gnome-music updates don't self-trigger notification changes. Eg. tagging a song or changing the usage counter triggering a change that gets notified, the current view/model reloaded, and the current song forgotten. The worst effect of all that is gnome-music being unable to pick a next song after the current one finishes, so might be enough to preserve the tracker:id of the current song and look it up in the new model after repopulating.
- Repopulating needs some cancellation love, currently too quick changes may result in doubly rows because previous async queries weren't cancelled.

Review of attachment 341147 [details] [review]:

lgtm

Review of attachment 341148 [details] [review]:

This is much better.

::: gnomemusic/grilo.py
@@ +120,2 @@
     @log
     def _on_content_changed(self, mediaSource, changedMedias, changeType, locationUnknown):

while working on this we might as well make it PEP8: media_source, changed_media, etc.
+ line length.

@@ +120,3 @@
     @log
     def _on_content_changed(self, mediaSource, changedMedias, changeType, locationUnknown):
         try:

I never understood this whole try/except block, I don't think anything here will throw an error?

Anyway, I always prefer just try/excepting the statement you expect the error from (if any).

@@ +124,3 @@
+
+            for media in changedMedias:
+                # Ignore non audio

I think this comment is obvious from the code and can be removed.

@@ +125,3 @@
+            for media in changedMedias:
+                # Ignore non audio
+                if not media.is_audio():

Using grilo is good, but this also means there's a query in query.py that is now unused, so should be removed with it.

@@ +138,3 @@
+                    self.deleted_media_ids.append(media_id)
+
+            if len(self.changed_media_ids) > 0 or len(self.deleted_media_ids) > 0:

this we usually indent as

if (<statement>
        or <statement>):

Aka: enclose multi statement if's with brackets and keep every separate statement on a new line with the operator as opener.

@@ +181,3 @@
                         self.content_changed_timeout = None
                         self.notification_handler = self.tracker.connect(
+                            'content-changed', self._on_content_changed)

I didn't think the rate limit in itself was a bad idea at this time (eventually depends on how well the widgets deal with a row of single updates).

Having a decently timed rate-limit would at least alleviate the problem with re-populates adding the same item several times over for now.

Review of attachment 341149 [details] [review]:

This would fix it for now.

In general I'm not quite happy with how views deal with this, it looks hackish to me.

Review of attachment 341150 [details] [review]:

We're not quite there yet (everything async), but that should be minor work. I was just waiting for the newcomers effort ;)

::: gnomemusic/views/artistsview.py
@@ +84,3 @@
             self._artists.clear()
             self._offset = 0
+            self._populate()

weird that this one calls _populate and the other populate.. maybe we should unify that too. I'm not quite sure why there is a distinction anyway.

Review of attachment 341150 [details] [review]:

I was wrong here, all calls should already be async now.

(In reply to Carlos Garnacho from comment #12)
> The main patch might be btw considered as kinda wip. There's some areas of
> improvement:
> - The notification handling collects the changed IDs, but those nothing with
> that info, everything is just queried again. This might be more or less ok,
> but

Eventually this is not ok, we should be using the IDs to update the model(s).

> - The handling of updates misses some worthwhile things, mainly so
> gnome-music updates don't self-trigger notification changes. Eg. tagging a
> song or changing the usage counter triggering a change that gets notified,
> the current view/model reloaded, and the current song forgotten. The worst
> effect of all that is gnome-music being unable to pick a next song after the
> current one finishes, so might be enough to preserve the tracker:id of the
> current song and look it up in the new model after repopulating.

Yeah, this is a major problem in dynamic playlists too. (#766244)

Again, we should eventually be updating models imo, not destroying and recreating them.

But for now a workaround involving the current ID should work.

> - Repopulating needs some cancellation love, currently too quick changes may
> result in doubly rows because previous async queries weren't cancelled.

Yeah, I don't think grilo queries atm support cancellation. But again this should also be solvable by not doing the destroy->recreate sequence

-- 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/gnome-music/issues/61.