Batch scrobble rule application by type to reduce N queries to 2 (#61)
* Implement optimized batch rule application - Add apply_all_album_rules/1 to batch apply all album rules in single query - Add apply_all_artist_rules/1 to batch apply all artist rules in single query - Update apply_all_rules/0 to use new batch functions - Add comprehensive tests for batch application - Use CASE statement in SQL to apply multiple rules efficiently Co-authored-by: cloud8421 <537608+cloud8421@users.noreply.github.com> * Add batch application support for track-filtered rules - Extend apply_all_album_rules/2 to support filtering by tracks - Extend apply_all_artist_rules/2 to support filtering by tracks - Update apply_all_rules/1 to use batch functions for track-filtered application - Remove duplicate function definitions - Ensures both main use cases (all tracks and specific tracks) are optimized Co-authored-by: cloud8421 <537608+cloud8421@users.noreply.github.com> * Add comprehensive documentation for optimization - Document the problem and solution approach - Explain SQL generation and performance impact - Detail trade-offs and backward compatibility - Include future considerations and scalability notes Co-authored-by: cloud8421 <537608+cloud8421@users.noreply.github.com> * Apply credo suggestions and format --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: cloud8421 <537608+cloud8421@users.noreply.github.com> Co-authored-by: Claudio Ortolina <cloud8421@gmail.com>
This commit is contained in:
@@ -0,0 +1,219 @@
|
|||||||
|
# Scrobble Rule Application Optimization
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
This document describes the optimization made to the scrobble rule application system to improve performance when applying multiple rules.
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
Previously, when applying all enabled scrobble rules, each rule was applied independently with a separate database UPDATE query. With N rules, this resulted in N separate database operations, which became inefficient as the number of rules grew.
|
||||||
|
|
||||||
|
### Previous Implementation
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
def apply_all_rules do
|
||||||
|
list_enabled_rules()
|
||||||
|
|> Enum.map(fn rule ->
|
||||||
|
case apply_rule(rule) do
|
||||||
|
{:ok, count} -> {:ok, {rule.type, rule.match_value, count}}
|
||||||
|
{:error, reason} -> {:error, {rule.type, rule.match_value, reason}}
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
This approach meant:
|
||||||
|
- 10 rules = 10 separate UPDATE queries
|
||||||
|
- 100 rules = 100 separate UPDATE queries
|
||||||
|
- Each query had to scan the entire `scrobbled_tracks` table
|
||||||
|
|
||||||
|
## Solution
|
||||||
|
|
||||||
|
The optimization groups rules by type (album/artist) and applies all rules of each type in a single database query using SQLite's CASE statement.
|
||||||
|
|
||||||
|
### New Implementation
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
def apply_all_rules do
|
||||||
|
enabled_rules = list_enabled_rules()
|
||||||
|
|
||||||
|
# Group rules by type
|
||||||
|
{album_rules, artist_rules} =
|
||||||
|
Enum.split_with(enabled_rules, fn rule -> rule.type == :album end)
|
||||||
|
|
||||||
|
# Apply all album rules in one query
|
||||||
|
apply_all_album_rules(album_rules)
|
||||||
|
|
||||||
|
# Apply all artist rules in one query
|
||||||
|
apply_all_artist_rules(artist_rules)
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
This approach means:
|
||||||
|
- 10 album rules + 10 artist rules = 2 total UPDATE queries (one for albums, one for artists)
|
||||||
|
- 100 album rules + 100 artist rules = 2 total UPDATE queries
|
||||||
|
- Each query still scans the table once, but updates all matching records in a single pass
|
||||||
|
|
||||||
|
## Technical Details
|
||||||
|
|
||||||
|
### SQL Generation
|
||||||
|
|
||||||
|
The optimization dynamically builds a CASE statement for all rules of the same type:
|
||||||
|
|
||||||
|
**Example for 2 album rules:**
|
||||||
|
|
||||||
|
```sql
|
||||||
|
UPDATE scrobbled_tracks
|
||||||
|
SET album = CASE
|
||||||
|
WHEN json_extract(album, '$.title') = 'Dark Side of the Moon'
|
||||||
|
THEN json_set(album, '$.musicbrainz_id', '12345678-1234-1234-1234-123456789012')
|
||||||
|
WHEN json_extract(album, '$.title') = 'Wish You Were Here'
|
||||||
|
THEN json_set(album, '$.musicbrainz_id', 'abcdef12-3456-7890-abcd-ef1234567890')
|
||||||
|
ELSE album
|
||||||
|
END
|
||||||
|
WHERE json_extract(album, '$.title') IN ('Dark Side of the Moon', 'Wish You Were Here')
|
||||||
|
```
|
||||||
|
|
||||||
|
The WHERE clause uses IN to filter only tracks that match any of the rules, avoiding unnecessary updates.
|
||||||
|
|
||||||
|
### Functions Added
|
||||||
|
|
||||||
|
#### 1. `apply_all_album_rules/1`
|
||||||
|
Applies all album rules in a single query.
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
@spec apply_all_album_rules([ScrobbleRule.t()]) :: {:ok, non_neg_integer()} | {:error, any()}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### 2. `apply_all_artist_rules/1`
|
||||||
|
Applies all artist rules in a single query.
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
@spec apply_all_artist_rules([ScrobbleRule.t()]) :: {:ok, non_neg_integer()} | {:error, any()}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### 3. `apply_all_album_rules/2`
|
||||||
|
Applies all album rules to a specific set of tracks in a single query.
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
@spec apply_all_album_rules([ScrobbleRule.t()], [Track.t()]) ::
|
||||||
|
{:ok, non_neg_integer()} | {:error, any()}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### 4. `apply_all_artist_rules/2`
|
||||||
|
Applies all artist rules to a specific set of tracks in a single query.
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
@spec apply_all_artist_rules([ScrobbleRule.t()], [Track.t()]) ::
|
||||||
|
{:ok, non_neg_integer()} | {:error, any()}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Use Cases
|
||||||
|
|
||||||
|
The optimization handles two main use cases:
|
||||||
|
|
||||||
|
#### 1. Periodic Full Application
|
||||||
|
The `ApplyScrobbleRules` Oban worker runs every 30 minutes and applies all rules to all tracks:
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
ScrobbleRules.apply_all_rules()
|
||||||
|
```
|
||||||
|
|
||||||
|
#### 2. Incremental Application on New Tracks
|
||||||
|
When new tracks are inserted via `LastFm.Feed.update/1`, rules are applied to just those tracks:
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
tracks
|
||||||
|
|> ScrobbleRules.apply_all_rules()
|
||||||
|
```
|
||||||
|
|
||||||
|
Both use cases now benefit from batched rule application.
|
||||||
|
|
||||||
|
## Performance Impact
|
||||||
|
|
||||||
|
### Expected Improvements
|
||||||
|
|
||||||
|
For a database with:
|
||||||
|
- 10,000 scrobbled tracks
|
||||||
|
- 20 enabled rules (10 album + 10 artist)
|
||||||
|
|
||||||
|
**Before:**
|
||||||
|
- 20 UPDATE queries
|
||||||
|
- Each query scans ~10,000 rows
|
||||||
|
- Total: ~200,000 row scans
|
||||||
|
|
||||||
|
**After:**
|
||||||
|
- 2 UPDATE queries (1 for albums, 1 for artists)
|
||||||
|
- Each query scans ~10,000 rows
|
||||||
|
- Total: ~20,000 row scans
|
||||||
|
|
||||||
|
**Result:** ~10x reduction in database operations for this scenario.
|
||||||
|
|
||||||
|
The benefit scales with the number of rules:
|
||||||
|
- 5 rules: ~2.5x improvement
|
||||||
|
- 10 rules: ~5x improvement
|
||||||
|
- 20 rules: ~10x improvement
|
||||||
|
- 100 rules: ~50x improvement
|
||||||
|
|
||||||
|
## Trade-offs
|
||||||
|
|
||||||
|
### Return Value Semantics
|
||||||
|
|
||||||
|
With the batched approach, all rules of the same type report the same aggregate count (total rows updated) rather than individual per-rule counts:
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
# Before: Each rule gets its own count
|
||||||
|
[
|
||||||
|
{:ok, {:album, "Album 1", 5}}, # 5 tracks updated
|
||||||
|
{:ok, {:album, "Album 2", 3}}, # 3 tracks updated
|
||||||
|
]
|
||||||
|
|
||||||
|
# After: Both rules report the total updated
|
||||||
|
[
|
||||||
|
{:ok, {:album, "Album 1", 8}}, # 8 total tracks updated
|
||||||
|
{:ok, {:album, "Album 2", 8}}, # 8 total tracks updated
|
||||||
|
]
|
||||||
|
```
|
||||||
|
|
||||||
|
This is an acceptable trade-off because:
|
||||||
|
1. The logging still shows how many rules were applied
|
||||||
|
2. The total tracks updated is more meaningful for understanding impact
|
||||||
|
3. Getting individual counts would require separate queries, negating the optimization
|
||||||
|
|
||||||
|
### Backward Compatibility
|
||||||
|
|
||||||
|
The API remains fully backward compatible:
|
||||||
|
- All existing functions (`apply_rule/1`, `apply_album_rule/1`, etc.) still work
|
||||||
|
- Return value format is unchanged: `{:ok, {type, match_value, count}}`
|
||||||
|
- Error handling is unchanged
|
||||||
|
- Tests continue to pass
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
New tests were added to verify the optimization:
|
||||||
|
|
||||||
|
1. `apply_all_album_rules/1` applies multiple album rules in one query
|
||||||
|
2. `apply_all_artist_rules/1` applies multiple artist rules in one query
|
||||||
|
3. Empty list handling returns `{:ok, 0}`
|
||||||
|
4. `apply_all_rules/0` correctly batches rules by type
|
||||||
|
5. All existing tests continue to pass
|
||||||
|
|
||||||
|
## Future Considerations
|
||||||
|
|
||||||
|
### Potential Enhancements
|
||||||
|
|
||||||
|
1. **Parallel Execution**: Album and artist rule applications could run in parallel since they're independent
|
||||||
|
2. **Query Optimization**: Could add indexes on `json_extract(album, '$.title')` and `json_extract(artist, '$.name')` if performance is still a concern
|
||||||
|
3. **Metrics**: Add telemetry events to track batch sizes and execution times
|
||||||
|
4. **Rule Ordering**: If rule priority matters in the future, the CASE statement naturally handles this (first match wins)
|
||||||
|
|
||||||
|
### Scalability
|
||||||
|
|
||||||
|
The current approach scales well up to ~100 rules per type. Beyond that, consider:
|
||||||
|
- SQL query length limits (SQLite's default limit is 1MB, which allows ~10,000 rules)
|
||||||
|
- Transaction size and memory usage
|
||||||
|
- Potential chunking if needed (apply rules in batches of N)
|
||||||
|
|
||||||
|
## Conclusion
|
||||||
|
|
||||||
|
This optimization significantly improves the performance of scrobble rule application by reducing database operations from O(N) to O(1) per rule type, where N is the number of rules. The implementation maintains backward compatibility while providing substantial performance benefits that scale with the number of rules.
|
||||||
@@ -326,9 +326,229 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
apply_artist_rule(rule, tracks)
|
apply_artist_rule(rule, tracks)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Applies all album rules in a single query.
|
||||||
|
|
||||||
|
Uses a CASE statement to update the musicbrainz_id for all matching albums
|
||||||
|
in one database operation, which is more efficient than applying rules individually.
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> apply_all_album_rules([rule1, rule2])
|
||||||
|
{:ok, 15}
|
||||||
|
|
||||||
|
"""
|
||||||
|
def apply_all_album_rules([]), do: {:ok, 0}
|
||||||
|
|
||||||
|
def apply_all_album_rules(rules) when is_list(rules) do
|
||||||
|
# Build CASE WHEN clauses dynamically
|
||||||
|
{case_clauses, case_params} =
|
||||||
|
rules
|
||||||
|
|> Enum.reduce({"", []}, fn rule, {sql_acc, params_acc} ->
|
||||||
|
clause =
|
||||||
|
"WHEN json_extract(album, '$.title') = ? THEN json_set(album, '$.musicbrainz_id', ?) "
|
||||||
|
|
||||||
|
{sql_acc <> clause, params_acc ++ [rule.match_value, rule.target_musicbrainz_id]}
|
||||||
|
end)
|
||||||
|
|
||||||
|
# Build complete UPDATE statement
|
||||||
|
case_sql = "CASE #{case_clauses}ELSE album END"
|
||||||
|
|
||||||
|
# Build WHERE IN clause
|
||||||
|
match_values = Enum.map(rules, & &1.match_value)
|
||||||
|
in_placeholders = Enum.map_join(match_values, ", ", fn _ -> "?" end)
|
||||||
|
where_sql = "json_extract(album, '$.title') IN (#{in_placeholders})"
|
||||||
|
|
||||||
|
# Complete SQL
|
||||||
|
sql = """
|
||||||
|
UPDATE scrobbled_tracks
|
||||||
|
SET album = #{case_sql}
|
||||||
|
WHERE #{where_sql}
|
||||||
|
"""
|
||||||
|
|
||||||
|
# All parameters: case params + where params
|
||||||
|
all_params = case_params ++ match_values
|
||||||
|
|
||||||
|
# Execute the query
|
||||||
|
case Repo.query(sql, all_params) do
|
||||||
|
{:ok, %{num_rows: count}} -> {:ok, count}
|
||||||
|
{:error, reason} -> {:error, reason}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Applies all album rules to a specific set of tracks in a single query.
|
||||||
|
|
||||||
|
Uses a CASE statement to update the musicbrainz_id for all matching albums
|
||||||
|
in one database operation, filtering by the provided tracks.
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> apply_all_album_rules([rule1, rule2], tracks)
|
||||||
|
{:ok, 3}
|
||||||
|
|
||||||
|
"""
|
||||||
|
def apply_all_album_rules([], _tracks), do: {:ok, 0}
|
||||||
|
def apply_all_album_rules(_rules, []), do: {:ok, 0}
|
||||||
|
|
||||||
|
def apply_all_album_rules(rules, tracks) when is_list(rules) and is_list(tracks) do
|
||||||
|
# Build CASE WHEN clauses dynamically
|
||||||
|
{case_clauses, case_params} =
|
||||||
|
rules
|
||||||
|
|> Enum.reduce({"", []}, fn rule, {sql_acc, params_acc} ->
|
||||||
|
clause =
|
||||||
|
"WHEN json_extract(album, '$.title') = ? THEN json_set(album, '$.musicbrainz_id', ?) "
|
||||||
|
|
||||||
|
{sql_acc <> clause, params_acc ++ [rule.match_value, rule.target_musicbrainz_id]}
|
||||||
|
end)
|
||||||
|
|
||||||
|
# Build complete UPDATE statement
|
||||||
|
case_sql = "CASE #{case_clauses}ELSE album END"
|
||||||
|
|
||||||
|
# Build WHERE IN clause for album titles
|
||||||
|
match_values = Enum.map(rules, & &1.match_value)
|
||||||
|
album_placeholders = Enum.map_join(match_values, ", ", fn _ -> "?" end)
|
||||||
|
|
||||||
|
# Build WHERE IN clause for track timestamps
|
||||||
|
track_scrobbled_at_uts = Enum.map(tracks, & &1.scrobbled_at_uts)
|
||||||
|
track_placeholders = Enum.map_join(track_scrobbled_at_uts, ", ", fn _ -> "?" end)
|
||||||
|
|
||||||
|
where_sql =
|
||||||
|
"json_extract(album, '$.title') IN (#{album_placeholders}) AND " <>
|
||||||
|
"scrobbled_at_uts IN (#{track_placeholders})"
|
||||||
|
|
||||||
|
# Complete SQL
|
||||||
|
sql = """
|
||||||
|
UPDATE scrobbled_tracks
|
||||||
|
SET album = #{case_sql}
|
||||||
|
WHERE #{where_sql}
|
||||||
|
"""
|
||||||
|
|
||||||
|
# All parameters: case params + album match values + track timestamps
|
||||||
|
all_params = case_params ++ match_values ++ track_scrobbled_at_uts
|
||||||
|
|
||||||
|
# Execute the query
|
||||||
|
case Repo.query(sql, all_params) do
|
||||||
|
{:ok, %{num_rows: count}} -> {:ok, count}
|
||||||
|
{:error, reason} -> {:error, reason}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Applies all artist rules in a single query.
|
||||||
|
|
||||||
|
Uses a CASE statement to update the musicbrainz_id for all matching artists
|
||||||
|
in one database operation, which is more efficient than applying rules individually.
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> apply_all_artist_rules([rule1, rule2])
|
||||||
|
{:ok, 25}
|
||||||
|
|
||||||
|
"""
|
||||||
|
def apply_all_artist_rules([]), do: {:ok, 0}
|
||||||
|
|
||||||
|
def apply_all_artist_rules(rules) when is_list(rules) do
|
||||||
|
# Build CASE WHEN clauses dynamically
|
||||||
|
{case_clauses, case_params} =
|
||||||
|
rules
|
||||||
|
|> Enum.reduce({"", []}, fn rule, {sql_acc, params_acc} ->
|
||||||
|
clause =
|
||||||
|
"WHEN json_extract(artist, '$.name') = ? THEN json_set(artist, '$.musicbrainz_id', ?) "
|
||||||
|
|
||||||
|
{sql_acc <> clause, params_acc ++ [rule.match_value, rule.target_musicbrainz_id]}
|
||||||
|
end)
|
||||||
|
|
||||||
|
# Build complete UPDATE statement
|
||||||
|
case_sql = "CASE #{case_clauses}ELSE artist END"
|
||||||
|
|
||||||
|
# Build WHERE IN clause
|
||||||
|
match_values = Enum.map(rules, & &1.match_value)
|
||||||
|
in_placeholders = Enum.map_join(match_values, ", ", fn _ -> "?" end)
|
||||||
|
where_sql = "json_extract(artist, '$.name') IN (#{in_placeholders})"
|
||||||
|
|
||||||
|
# Complete SQL
|
||||||
|
sql = """
|
||||||
|
UPDATE scrobbled_tracks
|
||||||
|
SET artist = #{case_sql}
|
||||||
|
WHERE #{where_sql}
|
||||||
|
"""
|
||||||
|
|
||||||
|
# All parameters: case params + where params
|
||||||
|
all_params = case_params ++ match_values
|
||||||
|
|
||||||
|
# Execute the query
|
||||||
|
case Repo.query(sql, all_params) do
|
||||||
|
{:ok, %{num_rows: count}} -> {:ok, count}
|
||||||
|
{:error, reason} -> {:error, reason}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Applies all artist rules to a specific set of tracks in a single query.
|
||||||
|
|
||||||
|
Uses a CASE statement to update the musicbrainz_id for all matching artists
|
||||||
|
in one database operation, filtering by the provided tracks.
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> apply_all_artist_rules([rule1, rule2], tracks)
|
||||||
|
{:ok, 7}
|
||||||
|
|
||||||
|
"""
|
||||||
|
def apply_all_artist_rules([], _tracks), do: {:ok, 0}
|
||||||
|
def apply_all_artist_rules(_rules, []), do: {:ok, 0}
|
||||||
|
|
||||||
|
def apply_all_artist_rules(rules, tracks) when is_list(rules) and is_list(tracks) do
|
||||||
|
# Build CASE WHEN clauses dynamically
|
||||||
|
{case_clauses, case_params} =
|
||||||
|
rules
|
||||||
|
|> Enum.reduce({"", []}, fn rule, {sql_acc, params_acc} ->
|
||||||
|
clause =
|
||||||
|
"WHEN json_extract(artist, '$.name') = ? THEN json_set(artist, '$.musicbrainz_id', ?) "
|
||||||
|
|
||||||
|
{sql_acc <> clause, params_acc ++ [rule.match_value, rule.target_musicbrainz_id]}
|
||||||
|
end)
|
||||||
|
|
||||||
|
# Build complete UPDATE statement
|
||||||
|
case_sql = "CASE #{case_clauses}ELSE artist END"
|
||||||
|
|
||||||
|
# Build WHERE IN clause for artist names
|
||||||
|
match_values = Enum.map(rules, & &1.match_value)
|
||||||
|
artist_placeholders = Enum.map_join(match_values, ", ", fn _ -> "?" end)
|
||||||
|
|
||||||
|
# Build WHERE IN clause for track timestamps
|
||||||
|
track_scrobbled_at_uts = Enum.map(tracks, & &1.scrobbled_at_uts)
|
||||||
|
track_placeholders = Enum.map_join(track_scrobbled_at_uts, ", ", fn _ -> "?" end)
|
||||||
|
|
||||||
|
where_sql =
|
||||||
|
"json_extract(artist, '$.name') IN (#{artist_placeholders}) AND " <>
|
||||||
|
"scrobbled_at_uts IN (#{track_placeholders})"
|
||||||
|
|
||||||
|
# Complete SQL
|
||||||
|
sql = """
|
||||||
|
UPDATE scrobbled_tracks
|
||||||
|
SET artist = #{case_sql}
|
||||||
|
WHERE #{where_sql}
|
||||||
|
"""
|
||||||
|
|
||||||
|
# All parameters: case params + artist match values + track timestamps
|
||||||
|
all_params = case_params ++ match_values ++ track_scrobbled_at_uts
|
||||||
|
|
||||||
|
# Execute the query
|
||||||
|
case Repo.query(sql, all_params) do
|
||||||
|
{:ok, %{num_rows: count}} -> {:ok, count}
|
||||||
|
{:error, reason} -> {:error, reason}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Applies all enabled rules.
|
Applies all enabled rules.
|
||||||
|
|
||||||
|
This optimized version groups rules by type and applies all rules of each type
|
||||||
|
in a single database query, which is much more efficient than applying each rule
|
||||||
|
individually.
|
||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|
||||||
iex> apply_all_rules()
|
iex> apply_all_rules()
|
||||||
@@ -337,14 +557,45 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
"""
|
"""
|
||||||
def apply_all_rules do
|
def apply_all_rules do
|
||||||
:telemetry.span([:music_library, :scrobble_rules, :apply_all_rules], %{}, fn ->
|
:telemetry.span([:music_library, :scrobble_rules, :apply_all_rules], %{}, fn ->
|
||||||
result =
|
enabled_rules = list_enabled_rules()
|
||||||
list_enabled_rules()
|
|
||||||
|> Enum.map(fn rule ->
|
# Group rules by type
|
||||||
case apply_rule(rule) do
|
{album_rules, artist_rules} =
|
||||||
{:ok, count} -> {:ok, {rule.type, rule.match_value, count}}
|
Enum.split_with(enabled_rules, fn rule -> rule.type == :album end)
|
||||||
{:error, reason} -> {:error, {rule.type, rule.match_value, reason}}
|
|
||||||
end
|
# Apply all album rules in one query
|
||||||
end)
|
album_result =
|
||||||
|
case apply_all_album_rules(album_rules) do
|
||||||
|
{:ok, count} ->
|
||||||
|
# Return the count for each album rule (total updated)
|
||||||
|
# Note: this returns the same count for each rule since they're applied together
|
||||||
|
Enum.map(album_rules, fn rule ->
|
||||||
|
{:ok, {rule.type, rule.match_value, count}}
|
||||||
|
end)
|
||||||
|
|
||||||
|
{:error, reason} ->
|
||||||
|
Enum.map(album_rules, fn rule ->
|
||||||
|
{:error, {rule.type, rule.match_value, reason}}
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
# Apply all artist rules in one query
|
||||||
|
artist_result =
|
||||||
|
case apply_all_artist_rules(artist_rules) do
|
||||||
|
{:ok, count} ->
|
||||||
|
# Return the count for each artist rule (total updated)
|
||||||
|
# Note: this returns the same count for each rule since they're applied together
|
||||||
|
Enum.map(artist_rules, fn rule ->
|
||||||
|
{:ok, {rule.type, rule.match_value, count}}
|
||||||
|
end)
|
||||||
|
|
||||||
|
{:error, reason} ->
|
||||||
|
Enum.map(artist_rules, fn rule ->
|
||||||
|
{:error, {rule.type, rule.match_value, reason}}
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
result = album_result ++ artist_result
|
||||||
|
|
||||||
{result, %{scrobble_track_count: :all}}
|
{result, %{scrobble_track_count: :all}}
|
||||||
end)
|
end)
|
||||||
@@ -353,6 +604,9 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
@doc """
|
@doc """
|
||||||
Applies all enabled rules to a specific set of tracks.
|
Applies all enabled rules to a specific set of tracks.
|
||||||
|
|
||||||
|
This optimized version groups rules by type and applies all rules of each type
|
||||||
|
in a single database query, filtering by the provided tracks.
|
||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|
||||||
iex> apply_all_rules(tracks)
|
iex> apply_all_rules(tracks)
|
||||||
@@ -368,14 +622,41 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
|
|
||||||
def apply_all_rules(tracks) do
|
def apply_all_rules(tracks) do
|
||||||
:telemetry.span([:music_library, :scrobble_rules, :apply_all_rules], %{}, fn ->
|
:telemetry.span([:music_library, :scrobble_rules, :apply_all_rules], %{}, fn ->
|
||||||
result =
|
enabled_rules = list_enabled_rules()
|
||||||
list_enabled_rules()
|
|
||||||
|> Enum.map(fn rule ->
|
# Group rules by type
|
||||||
case apply_rule(rule, tracks) do
|
{album_rules, artist_rules} =
|
||||||
{:ok, count} -> {:ok, {rule.type, rule.match_value, count}}
|
Enum.split_with(enabled_rules, fn rule -> rule.type == :album end)
|
||||||
{:error, reason} -> {:error, {rule.type, rule.match_value, reason}}
|
|
||||||
end
|
# Apply all album rules in one query
|
||||||
end)
|
album_result =
|
||||||
|
case apply_all_album_rules(album_rules, tracks) do
|
||||||
|
{:ok, count} ->
|
||||||
|
Enum.map(album_rules, fn rule ->
|
||||||
|
{:ok, {rule.type, rule.match_value, count}}
|
||||||
|
end)
|
||||||
|
|
||||||
|
{:error, reason} ->
|
||||||
|
Enum.map(album_rules, fn rule ->
|
||||||
|
{:error, {rule.type, rule.match_value, reason}}
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
# Apply all artist rules in one query
|
||||||
|
artist_result =
|
||||||
|
case apply_all_artist_rules(artist_rules, tracks) do
|
||||||
|
{:ok, count} ->
|
||||||
|
Enum.map(artist_rules, fn rule ->
|
||||||
|
{:ok, {rule.type, rule.match_value, count}}
|
||||||
|
end)
|
||||||
|
|
||||||
|
{:error, reason} ->
|
||||||
|
Enum.map(artist_rules, fn rule ->
|
||||||
|
{:error, {rule.type, rule.match_value, reason}}
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
result = album_result ++ artist_result
|
||||||
|
|
||||||
{result, %{scrobble_track_count: Enum.count(tracks)}}
|
{result, %{scrobble_track_count: Enum.count(tracks)}}
|
||||||
end)
|
end)
|
||||||
|
|||||||
@@ -287,5 +287,141 @@ defmodule MusicLibrary.ScrobbleRulesTest do
|
|||||||
|
|
||||||
assert ScrobbleRules.count_artist_matches(rule) == 2
|
assert ScrobbleRules.count_artist_matches(rule) == 2
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "apply_all_album_rules/1 applies multiple album rules in one query" do
|
||||||
|
# Create two album rules
|
||||||
|
rule1 = scrobble_rule_fixture(@valid_album_attrs)
|
||||||
|
|
||||||
|
rule2 =
|
||||||
|
scrobble_rule_fixture(%{
|
||||||
|
match_value: "Wish You Were Here",
|
||||||
|
target_musicbrainz_id: "abcdef12-3456-7890-abcd-ef1234567890"
|
||||||
|
})
|
||||||
|
|
||||||
|
# Create tracks matching each rule
|
||||||
|
track1 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
album: %{musicbrainz_id: "", title: "Dark Side of the Moon"}
|
||||||
|
})
|
||||||
|
|
||||||
|
track2 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
scrobbled_at_uts: System.system_time(:second) + 1,
|
||||||
|
album: %{musicbrainz_id: "", title: "Wish You Were Here"}
|
||||||
|
})
|
||||||
|
|
||||||
|
# Create a non-matching track
|
||||||
|
_track3 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
scrobbled_at_uts: System.system_time(:second) + 2,
|
||||||
|
album: %{musicbrainz_id: "", title: "The Wall"}
|
||||||
|
})
|
||||||
|
|
||||||
|
# Apply both rules at once
|
||||||
|
assert {:ok, 2} = ScrobbleRules.apply_all_album_rules([rule1, rule2])
|
||||||
|
|
||||||
|
# Verify both tracks were updated by fetching them again
|
||||||
|
updated_track1 = Repo.get(Track, track1.scrobbled_at_uts)
|
||||||
|
assert updated_track1.album.musicbrainz_id == rule1.target_musicbrainz_id
|
||||||
|
|
||||||
|
updated_track2 = Repo.get(Track, track2.scrobbled_at_uts)
|
||||||
|
assert updated_track2.album.musicbrainz_id == rule2.target_musicbrainz_id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "apply_all_artist_rules/1 applies multiple artist rules in one query" do
|
||||||
|
# Create two artist rules
|
||||||
|
rule1 = scrobble_rule_fixture(@valid_artist_attrs)
|
||||||
|
|
||||||
|
rule2 =
|
||||||
|
scrobble_rule_fixture(%{
|
||||||
|
type: :artist,
|
||||||
|
match_value: "Led Zeppelin",
|
||||||
|
target_musicbrainz_id: "fedcba98-7654-3210-fedc-ba9876543210"
|
||||||
|
})
|
||||||
|
|
||||||
|
# Create tracks matching each rule
|
||||||
|
track1 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
artist: %{musicbrainz_id: "", name: "Pink Floyd"}
|
||||||
|
})
|
||||||
|
|
||||||
|
track2 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
scrobbled_at_uts: System.system_time(:second) + 1,
|
||||||
|
artist: %{musicbrainz_id: "", name: "Led Zeppelin"}
|
||||||
|
})
|
||||||
|
|
||||||
|
# Create a non-matching track
|
||||||
|
_track3 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
scrobbled_at_uts: System.system_time(:second) + 2,
|
||||||
|
artist: %{musicbrainz_id: "", name: "The Beatles"}
|
||||||
|
})
|
||||||
|
|
||||||
|
# Apply both rules at once
|
||||||
|
assert {:ok, 2} = ScrobbleRules.apply_all_artist_rules([rule1, rule2])
|
||||||
|
|
||||||
|
# Verify both tracks were updated by fetching them again
|
||||||
|
updated_track1 = Repo.get(Track, track1.scrobbled_at_uts)
|
||||||
|
assert updated_track1.artist.musicbrainz_id == rule1.target_musicbrainz_id
|
||||||
|
|
||||||
|
updated_track2 = Repo.get(Track, track2.scrobbled_at_uts)
|
||||||
|
assert updated_track2.artist.musicbrainz_id == rule2.target_musicbrainz_id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "apply_all_album_rules/1 with empty list returns 0" do
|
||||||
|
assert {:ok, 0} = ScrobbleRules.apply_all_album_rules([])
|
||||||
|
end
|
||||||
|
|
||||||
|
test "apply_all_artist_rules/1 with empty list returns 0" do
|
||||||
|
assert {:ok, 0} = ScrobbleRules.apply_all_artist_rules([])
|
||||||
|
end
|
||||||
|
|
||||||
|
test "apply_all_rules/0 batches rules by type" do
|
||||||
|
# Create multiple rules of each type
|
||||||
|
_album_rule1 = scrobble_rule_fixture(@valid_album_attrs)
|
||||||
|
|
||||||
|
_album_rule2 =
|
||||||
|
scrobble_rule_fixture(%{
|
||||||
|
match_value: "Wish You Were Here",
|
||||||
|
target_musicbrainz_id: "abcdef12-3456-7890-abcd-ef1234567890"
|
||||||
|
})
|
||||||
|
|
||||||
|
_artist_rule1 = scrobble_rule_fixture(@valid_artist_attrs)
|
||||||
|
|
||||||
|
_artist_rule2 =
|
||||||
|
scrobble_rule_fixture(%{
|
||||||
|
type: :artist,
|
||||||
|
match_value: "Led Zeppelin",
|
||||||
|
target_musicbrainz_id: "fedcba98-7654-3210-fedc-ba9876543210"
|
||||||
|
})
|
||||||
|
|
||||||
|
# Create tracks matching the rules
|
||||||
|
_track1 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
album: %{musicbrainz_id: "", title: "Dark Side of the Moon"},
|
||||||
|
artist: %{musicbrainz_id: "", name: "Pink Floyd"}
|
||||||
|
})
|
||||||
|
|
||||||
|
_track2 =
|
||||||
|
scrobbled_track_fixture(%{
|
||||||
|
scrobbled_at_uts: System.system_time(:second) + 1,
|
||||||
|
album: %{musicbrainz_id: "", title: "Wish You Were Here"},
|
||||||
|
artist: %{musicbrainz_id: "", name: "Led Zeppelin"}
|
||||||
|
})
|
||||||
|
|
||||||
|
# Apply all rules
|
||||||
|
results = ScrobbleRules.apply_all_rules()
|
||||||
|
|
||||||
|
# Should have 4 results (one for each rule)
|
||||||
|
assert length(results) == 4
|
||||||
|
|
||||||
|
# All results should be successful
|
||||||
|
Enum.each(results, fn result ->
|
||||||
|
assert {:ok, {_type, _match_value, count}} = result
|
||||||
|
# Count should be > 0 since we have matching tracks
|
||||||
|
assert count > 0
|
||||||
|
end)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user