Deduplicate tracks in ListeningStats queries
When multiple records share MusicBrainz release IDs, the left joins in ListeningStats fan out — producing duplicate rows per scrobbled track. Fix by grouping release subqueries on release_id with MIN(record_id) before joining, so each release maps to exactly one record.
This commit is contained in:
@@ -269,9 +269,9 @@ defmodule MusicLibrary.ListeningStats do
|
|||||||
distinct: true
|
distinct: true
|
||||||
|
|
||||||
from t in Track,
|
from t in Track,
|
||||||
left_join: cr in subquery(Collection.collected_releases_query()),
|
left_join: cr in subquery(unique_collected_releases_query()),
|
||||||
on: cr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
on: cr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
||||||
left_join: wr in subquery(Wishlist.wishlisted_releases_query()),
|
left_join: wr in subquery(unique_wishlisted_releases_query()),
|
||||||
on: wr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
on: wr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
||||||
left_join: ar in subquery(all_artists_query),
|
left_join: ar in subquery(all_artists_query),
|
||||||
on: wr.record_id == ar.record_id or cr.record_id == ar.record_id,
|
on: wr.record_id == ar.record_id or cr.record_id == ar.record_id,
|
||||||
@@ -286,9 +286,9 @@ defmodule MusicLibrary.ListeningStats do
|
|||||||
|
|
||||||
defp top_albums_base_query do
|
defp top_albums_base_query do
|
||||||
from t in Track,
|
from t in Track,
|
||||||
left_join: cr in subquery(Collection.collected_releases_query()),
|
left_join: cr in subquery(unique_collected_releases_query()),
|
||||||
on: cr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
on: cr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
||||||
left_join: wr in subquery(Wishlist.wishlisted_releases_query()),
|
left_join: wr in subquery(unique_wishlisted_releases_query()),
|
||||||
on: wr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
on: wr.release_id == fragment("? ->> '$.musicbrainz_id'", t.album),
|
||||||
where: fragment("json_extract(album, '$.title') != ''"),
|
where: fragment("json_extract(album, '$.title') != ''"),
|
||||||
group_by: [
|
group_by: [
|
||||||
@@ -325,6 +325,26 @@ defmodule MusicLibrary.ListeningStats do
|
|||||||
order_by: [desc: count(t.scrobbled_at_uts, :distinct)]
|
order_by: [desc: count(t.scrobbled_at_uts, :distinct)]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp unique_collected_releases_query do
|
||||||
|
from rr in subquery(Collection.collected_releases_query()),
|
||||||
|
group_by: rr.release_id,
|
||||||
|
select: %{
|
||||||
|
record_id: min(rr.record_id),
|
||||||
|
cover_hash: min(rr.cover_hash),
|
||||||
|
release_id: rr.release_id
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
defp unique_wishlisted_releases_query do
|
||||||
|
from rr in subquery(Wishlist.wishlisted_releases_query()),
|
||||||
|
group_by: rr.release_id,
|
||||||
|
select: %{
|
||||||
|
record_id: min(rr.record_id),
|
||||||
|
cover_hash: min(rr.cover_hash),
|
||||||
|
release_id: rr.release_id
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
defp cutoff_timestamp(days, opts) do
|
defp cutoff_timestamp(days, opts) do
|
||||||
current_time = Keyword.get_lazy(opts, :current_time, &DateTime.utc_now/0)
|
current_time = Keyword.get_lazy(opts, :current_time, &DateTime.utc_now/0)
|
||||||
timezone = Keyword.get(opts, :timezone, &MusicLibrary.default_timezone/0)
|
timezone = Keyword.get(opts, :timezone, &MusicLibrary.default_timezone/0)
|
||||||
|
|||||||
@@ -286,6 +286,82 @@ defmodule MusicLibrary.ListeningStatsTest do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "deduplication when multiple records share release IDs" do
|
||||||
|
setup do
|
||||||
|
# Create two collected records that share the same MusicBrainz release IDs
|
||||||
|
# (both use the default marbles fixture with identical musicbrainz_data).
|
||||||
|
# This produces two rows per release_id in record_releases, which previously
|
||||||
|
# caused fan-out in ListeningStats joins.
|
||||||
|
record_a = RecordsFixtures.record(%{title: "Marbles Copy A"})
|
||||||
|
record_b = RecordsFixtures.record(%{title: "Marbles Copy B"})
|
||||||
|
|
||||||
|
# Pick a release_id shared by both records
|
||||||
|
shared_release_id = "d3f9b9e2-73f5-4b47-a2a7-2c2199aad608"
|
||||||
|
now = System.system_time(:second)
|
||||||
|
|
||||||
|
track_fixture(%{
|
||||||
|
title: "The Invisible Man",
|
||||||
|
album_title: "Marbles",
|
||||||
|
album_musicbrainz_id: shared_release_id,
|
||||||
|
artist_name: "Marillion",
|
||||||
|
scrobbled_at_uts: now - 100
|
||||||
|
})
|
||||||
|
|
||||||
|
track_fixture(%{
|
||||||
|
title: "You're Gone",
|
||||||
|
album_title: "Marbles",
|
||||||
|
album_musicbrainz_id: shared_release_id,
|
||||||
|
artist_name: "Marillion",
|
||||||
|
scrobbled_at_uts: now - 200
|
||||||
|
})
|
||||||
|
|
||||||
|
# The lexicographically smaller record_id should consistently win (MIN)
|
||||||
|
expected_record_id = Enum.min([record_a.id, record_b.id])
|
||||||
|
|
||||||
|
%{
|
||||||
|
expected_record_id: expected_record_id,
|
||||||
|
shared_release_id: shared_release_id
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
test "list_tracks returns one row per track", %{expected_record_id: expected_record_id} do
|
||||||
|
tracks = ListeningStats.list_tracks()
|
||||||
|
|
||||||
|
titles = Enum.map(tracks, & &1.track.title)
|
||||||
|
assert length(titles) == length(Enum.uniq(titles)), "list_tracks returned duplicate rows"
|
||||||
|
|
||||||
|
# Both tracks should map to the same deterministic record_id
|
||||||
|
for result <- tracks do
|
||||||
|
assert result.collected_record_id == expected_record_id
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "recent_activity returns one row per track", %{expected_record_id: expected_record_id} do
|
||||||
|
%{recent_tracks: recent_tracks} =
|
||||||
|
ListeningStats.recent_activity("Etc/UTC", 20)
|
||||||
|
|
||||||
|
titles = Enum.map(recent_tracks, & &1.track.title)
|
||||||
|
|
||||||
|
assert length(titles) == length(Enum.uniq(titles)),
|
||||||
|
"recent_activity returned duplicate rows"
|
||||||
|
|
||||||
|
for result <- recent_tracks do
|
||||||
|
assert result.collected_record_id == expected_record_id
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "get_top_albums returns one entry per album", %{expected_record_id: expected_record_id} do
|
||||||
|
results = ListeningStats.get_top_albums(limit: 10)
|
||||||
|
|
||||||
|
marbles_entries = Enum.filter(results, fn r -> r.album_title == "Marbles" end)
|
||||||
|
assert length(marbles_entries) == 1, "get_top_albums returned duplicate album entries"
|
||||||
|
|
||||||
|
[entry] = marbles_entries
|
||||||
|
assert entry.play_count == 2
|
||||||
|
assert entry.collected_record_id == expected_record_id
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "get_top_artists/1" do
|
describe "get_top_artists/1" do
|
||||||
test "counts tracks with missing artist_infos records" do
|
test "counts tracks with missing artist_infos records" do
|
||||||
artist_info =
|
artist_info =
|
||||||
|
|||||||
Reference in New Issue
Block a user