fix(server): fix case-insensitive sort order and add indexes to improve performance (#3425)

* refactor(server): better sort mappings

* refactor(server): simplify GetIndex

* fix: recreate tables and indexes using proper collation

Also add tests to ensure proper collation

* chore: remove unused method

* fix: sort expressions

* fix: lint errors

* fix: cleanup
This commit is contained in:
Deluan Quintão
2024-10-26 14:06:34 -04:00
committed by GitHub
parent 154e13f7c9
commit fcb5e1b806
18 changed files with 861 additions and 271 deletions
@@ -0,0 +1,512 @@
-- +goose Up
--region Artist Table
create table artist_dg_tmp
(
id varchar(255) not null
primary key,
name varchar(255) default '' not null,
album_count integer default 0 not null,
full_text varchar(255) default '',
song_count integer default 0 not null,
size integer default 0 not null,
biography varchar(255) default '' not null,
small_image_url varchar(255) default '' not null,
medium_image_url varchar(255) default '' not null,
large_image_url varchar(255) default '' not null,
similar_artists varchar(255) default '' not null,
external_url varchar(255) default '' not null,
external_info_updated_at datetime,
order_artist_name varchar collate NOCASE default '' not null,
sort_artist_name varchar collate NOCASE default '' not null,
mbz_artist_id varchar default '' not null
);
insert into artist_dg_tmp(id, name, album_count, full_text, song_count, size, biography, small_image_url,
medium_image_url, large_image_url, similar_artists, external_url, external_info_updated_at,
order_artist_name, sort_artist_name, mbz_artist_id)
select id,
name,
album_count,
full_text,
song_count,
size,
biography,
small_image_url,
medium_image_url,
large_image_url,
similar_artists,
external_url,
external_info_updated_at,
order_artist_name,
sort_artist_name,
mbz_artist_id
from artist;
drop table artist;
alter table artist_dg_tmp
rename to artist;
create index artist_full_text
on artist (full_text);
create index artist_name
on artist (name);
create index artist_order_artist_name
on artist (order_artist_name);
create index artist_size
on artist (size);
create index artist_sort_name
on artist (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NOCASE);
--endregion
--region Album Table
create table album_dg_tmp
(
id varchar(255) not null
primary key,
name varchar(255) default '' not null,
artist_id varchar(255) default '' not null,
embed_art_path varchar(255) default '' not null,
artist varchar(255) default '' not null,
album_artist varchar(255) default '' not null,
min_year int default 0 not null,
max_year integer default 0 not null,
compilation bool default FALSE not null,
song_count integer default 0 not null,
duration real default 0 not null,
genre varchar(255) default '' not null,
created_at datetime,
updated_at datetime,
full_text varchar(255) default '',
album_artist_id varchar(255) default '',
size integer default 0 not null,
all_artist_ids varchar,
description varchar(255) default '' not null,
small_image_url varchar(255) default '' not null,
medium_image_url varchar(255) default '' not null,
large_image_url varchar(255) default '' not null,
external_url varchar(255) default '' not null,
external_info_updated_at datetime,
date varchar(255) default '' not null,
min_original_year int default 0 not null,
max_original_year int default 0 not null,
original_date varchar(255) default '' not null,
release_date varchar(255) default '' not null,
releases integer default 0 not null,
image_files varchar default '' not null,
order_album_name varchar collate NOCASE default '' not null,
order_album_artist_name varchar collate NOCASE default '' not null,
sort_album_name varchar collate NOCASE default '' not null,
sort_album_artist_name varchar collate NOCASE default '' not null,
catalog_num varchar default '' not null,
comment varchar default '' not null,
paths varchar default '' not null,
mbz_album_id varchar default '' not null,
mbz_album_artist_id varchar default '' not null,
mbz_album_type varchar default '' not null,
mbz_album_comment varchar default '' not null,
discs jsonb default '{}' not null,
library_id integer default 1 not null
references library
on delete cascade
);
insert into album_dg_tmp(id, name, artist_id, embed_art_path, artist, album_artist, min_year, max_year, compilation,
song_count, duration, genre, created_at, updated_at, full_text, album_artist_id, size,
all_artist_ids, description, small_image_url, medium_image_url, large_image_url, external_url,
external_info_updated_at, date, min_original_year, max_original_year, original_date,
release_date, releases, image_files, order_album_name, order_album_artist_name,
sort_album_name, sort_album_artist_name, catalog_num, comment, paths,
mbz_album_id, mbz_album_artist_id, mbz_album_type, mbz_album_comment, discs, library_id)
select id,
name,
artist_id,
embed_art_path,
artist,
album_artist,
min_year,
max_year,
compilation,
song_count,
duration,
genre,
created_at,
updated_at,
full_text,
album_artist_id,
size,
all_artist_ids,
description,
small_image_url,
medium_image_url,
large_image_url,
external_url,
external_info_updated_at,
date,
min_original_year,
max_original_year,
original_date,
release_date,
releases,
image_files,
order_album_name,
order_album_artist_name,
sort_album_name,
sort_album_artist_name,
catalog_num,
comment,
paths,
mbz_album_id,
mbz_album_artist_id,
mbz_album_type,
mbz_album_comment,
discs,
library_id
from album;
drop table album;
alter table album_dg_tmp
rename to album;
create index album_all_artist_ids
on album (all_artist_ids);
create index album_alphabetical_by_artist
on album (compilation, order_album_artist_name, order_album_name);
create index album_artist
on album (artist);
create index album_artist_album
on album (artist);
create index album_artist_album_id
on album (album_artist_id);
create index album_artist_id
on album (artist_id);
create index album_created_at
on album (created_at);
create index album_full_text
on album (full_text);
create index album_genre
on album (genre);
create index album_max_year
on album (max_year);
create index album_mbz_album_type
on album (mbz_album_type);
create index album_min_year
on album (min_year);
create index album_name
on album (name);
create index album_order_album_artist_name
on album (order_album_artist_name);
create index album_order_album_name
on album (order_album_name);
create index album_size
on album (size);
create index album_sort_name
on album (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE);
create index album_sort_album_artist_name
on album (coalesce(nullif(sort_album_artist_name,''),order_album_artist_name) collate NOCASE);
create index album_updated_at
on album (updated_at);
--endregion
--region Media File Table
create table media_file_dg_tmp
(
id varchar(255) not null
primary key,
path varchar(255) default '' not null,
title varchar(255) default '' not null,
album varchar(255) default '' not null,
artist varchar(255) default '' not null,
artist_id varchar(255) default '' not null,
album_artist varchar(255) default '' not null,
album_id varchar(255) default '' not null,
has_cover_art bool default FALSE not null,
track_number integer default 0 not null,
disc_number integer default 0 not null,
year integer default 0 not null,
size integer default 0 not null,
suffix varchar(255) default '' not null,
duration real default 0 not null,
bit_rate integer default 0 not null,
genre varchar(255) default '' not null,
compilation bool default FALSE not null,
created_at datetime,
updated_at datetime,
full_text varchar(255) default '',
album_artist_id varchar(255) default '',
date varchar(255) default '' not null,
original_year int default 0 not null,
original_date varchar(255) default '' not null,
release_year int default 0 not null,
release_date varchar(255) default '' not null,
order_album_name varchar collate NOCASE default '' not null,
order_album_artist_name varchar collate NOCASE default '' not null,
order_artist_name varchar collate NOCASE default '' not null,
sort_album_name varchar collate NOCASE default '' not null,
sort_artist_name varchar collate NOCASE default '' not null,
sort_album_artist_name varchar collate NOCASE default '' not null,
sort_title varchar collate NOCASE default '' not null,
disc_subtitle varchar default '' not null,
catalog_num varchar default '' not null,
comment varchar default '' not null,
order_title varchar collate NOCASE default '' not null,
mbz_recording_id varchar default '' not null,
mbz_album_id varchar default '' not null,
mbz_artist_id varchar default '' not null,
mbz_album_artist_id varchar default '' not null,
mbz_album_type varchar default '' not null,
mbz_album_comment varchar default '' not null,
mbz_release_track_id varchar default '' not null,
bpm integer default 0 not null,
channels integer default 0 not null,
rg_album_gain real default 0 not null,
rg_album_peak real default 0 not null,
rg_track_gain real default 0 not null,
rg_track_peak real default 0 not null,
lyrics jsonb default '[]' not null,
sample_rate integer default 0 not null,
library_id integer default 1 not null
references library
on delete cascade
);
insert into media_file_dg_tmp(id, path, title, album, artist, artist_id, album_artist, album_id, has_cover_art,
track_number, disc_number, year, size, suffix, duration, bit_rate, genre, compilation,
created_at, updated_at, full_text, album_artist_id, date, original_year, original_date,
release_year, release_date, order_album_name, order_album_artist_name, order_artist_name,
sort_album_name, sort_artist_name, sort_album_artist_name, sort_title, disc_subtitle,
catalog_num, comment, order_title, mbz_recording_id, mbz_album_id, mbz_artist_id,
mbz_album_artist_id, mbz_album_type, mbz_album_comment, mbz_release_track_id, bpm,
channels, rg_album_gain, rg_album_peak, rg_track_gain, rg_track_peak, lyrics, sample_rate,
library_id)
select id,
path,
title,
album,
artist,
artist_id,
album_artist,
album_id,
has_cover_art,
track_number,
disc_number,
year,
size,
suffix,
duration,
bit_rate,
genre,
compilation,
created_at,
updated_at,
full_text,
album_artist_id,
date,
original_year,
original_date,
release_year,
release_date,
order_album_name,
order_album_artist_name,
order_artist_name,
sort_album_name,
sort_artist_name,
sort_album_artist_name,
sort_title,
disc_subtitle,
catalog_num,
comment,
order_title,
mbz_recording_id,
mbz_album_id,
mbz_artist_id,
mbz_album_artist_id,
mbz_album_type,
mbz_album_comment,
mbz_release_track_id,
bpm,
channels,
rg_album_gain,
rg_album_peak,
rg_track_gain,
rg_track_peak,
lyrics,
sample_rate,
library_id
from media_file;
drop table media_file;
alter table media_file_dg_tmp
rename to media_file;
create index media_file_album_artist
on media_file (album_artist);
create index media_file_album_id
on media_file (album_id);
create index media_file_artist
on media_file (artist);
create index media_file_artist_album_id
on media_file (album_artist_id);
create index media_file_artist_id
on media_file (artist_id);
create index media_file_bpm
on media_file (bpm);
create index media_file_channels
on media_file (channels);
create index media_file_created_at
on media_file (created_at);
create index media_file_duration
on media_file (duration);
create index media_file_full_text
on media_file (full_text);
create index media_file_genre
on media_file (genre);
create index media_file_mbz_track_id
on media_file (mbz_recording_id);
create index media_file_order_album_name
on media_file (order_album_name);
create index media_file_order_artist_name
on media_file (order_artist_name);
create index media_file_order_title
on media_file (order_title);
create index media_file_path
on media_file (path);
create index media_file_path_nocase
on media_file (path collate NOCASE);
create index media_file_sample_rate
on media_file (sample_rate);
create index media_file_sort_title
on media_file (coalesce(nullif(sort_title,''),order_title) collate NOCASE);
create index media_file_sort_artist_name
on media_file (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NOCASE);
create index media_file_sort_album_name
on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE);
create index media_file_title
on media_file (title);
create index media_file_track_number
on media_file (disc_number, track_number);
create index media_file_updated_at
on media_file (updated_at);
create index media_file_year
on media_file (year);
--endregion
--region Radio Table
create table radio_dg_tmp
(
id varchar(255) not null
primary key,
name varchar collate NOCASE not null
unique,
stream_url varchar not null,
home_page_url varchar default '' not null,
created_at datetime,
updated_at datetime
);
insert into radio_dg_tmp(id, name, stream_url, home_page_url, created_at, updated_at)
select id, name, stream_url, home_page_url, created_at, updated_at
from radio;
drop table radio;
alter table radio_dg_tmp
rename to radio;
create index radio_name
on radio(name);
--endregion
--region users Table
create table user_dg_tmp
(
id varchar(255) not null
primary key,
user_name varchar(255) default '' not null
unique,
name varchar(255) collate NOCASE default '' not null,
email varchar(255) default '' not null,
password varchar(255) default '' not null,
is_admin bool default FALSE not null,
last_login_at datetime,
last_access_at datetime,
created_at datetime not null,
updated_at datetime not null
);
insert into user_dg_tmp(id, user_name, name, email, password, is_admin, last_login_at, last_access_at, created_at,
updated_at)
select id,
user_name,
name,
email,
password,
is_admin,
last_login_at,
last_access_at,
created_at,
updated_at
from user;
drop table user;
alter table user_dg_tmp
rename to user;
create index user_username_password
on user(user_name collate NOCASE, password);
--endregion
-- +goose Down
alter table album
add column sort_artist_name varchar default '' not null;
-1
View File
@@ -38,7 +38,6 @@ type Album struct {
Discs Discs `structs:"discs" json:"discs,omitempty"` Discs Discs `structs:"discs" json:"discs,omitempty"`
FullText string `structs:"full_text" json:"-"` FullText string `structs:"full_text" json:"-"`
SortAlbumName string `structs:"sort_album_name" json:"sortAlbumName,omitempty"` SortAlbumName string `structs:"sort_album_name" json:"sortAlbumName,omitempty"`
SortArtistName string `structs:"sort_artist_name" json:"sortArtistName,omitempty"`
SortAlbumArtistName string `structs:"sort_album_artist_name" json:"sortAlbumArtistName,omitempty"` SortAlbumArtistName string `structs:"sort_album_artist_name" json:"sortAlbumArtistName,omitempty"`
OrderAlbumName string `structs:"order_album_name" json:"orderAlbumName"` OrderAlbumName string `structs:"order_album_name" json:"orderAlbumName"`
OrderAlbumArtistName string `structs:"order_album_artist_name" json:"orderAlbumArtistName"` OrderAlbumArtistName string `structs:"order_album_artist_name" json:"orderAlbumArtistName"`
+1 -3
View File
@@ -140,7 +140,6 @@ func (mfs MediaFiles) ToAlbum() Album {
a.AlbumArtist = m.AlbumArtist a.AlbumArtist = m.AlbumArtist
a.AlbumArtistID = m.AlbumArtistID a.AlbumArtistID = m.AlbumArtistID
a.SortAlbumName = m.SortAlbumName a.SortAlbumName = m.SortAlbumName
a.SortArtistName = m.SortArtistName
a.SortAlbumArtistName = m.SortAlbumArtistName a.SortAlbumArtistName = m.SortAlbumArtistName
a.OrderAlbumName = m.OrderAlbumName a.OrderAlbumName = m.OrderAlbumName
a.OrderAlbumArtistName = m.OrderAlbumArtistName a.OrderAlbumArtistName = m.OrderAlbumArtistName
@@ -261,11 +260,10 @@ type MediaFileRepository interface {
GetAll(options ...QueryOptions) (MediaFiles, error) GetAll(options ...QueryOptions) (MediaFiles, error)
Search(q string, offset int, size int) (MediaFiles, error) Search(q string, offset int, size int) (MediaFiles, error)
Delete(id string) error Delete(id string) error
FindByPaths(paths []string) (MediaFiles, error)
// Queries by path to support the scanner, no Annotations or Bookmarks required in the response // Queries by path to support the scanner, no Annotations or Bookmarks required in the response
FindAllByPath(path string) (MediaFiles, error) FindAllByPath(path string) (MediaFiles, error)
FindByPath(path string) (*MediaFile, error)
FindByPaths(paths []string) (MediaFiles, error)
FindPathsRecursively(basePath string) ([]string, error) FindPathsRecursively(basePath string) ([]string, error)
DeleteByPath(path string) (int64, error) DeleteByPath(path string) (int64, error)
-1
View File
@@ -43,7 +43,6 @@ var _ = Describe("MediaFiles", func() {
Expect(album.AlbumArtist).To(Equal("AlbumArtist")) Expect(album.AlbumArtist).To(Equal("AlbumArtist"))
Expect(album.AlbumArtistID).To(Equal("AlbumArtistID")) Expect(album.AlbumArtistID).To(Equal("AlbumArtistID"))
Expect(album.SortAlbumName).To(Equal("SortAlbumName")) Expect(album.SortAlbumName).To(Equal("SortAlbumName"))
Expect(album.SortArtistName).To(Equal("SortArtistName"))
Expect(album.SortAlbumArtistName).To(Equal("SortAlbumArtistName")) Expect(album.SortAlbumArtistName).To(Equal("SortAlbumArtistName"))
Expect(album.OrderAlbumName).To(Equal("OrderAlbumName")) Expect(album.OrderAlbumName).To(Equal("OrderAlbumName"))
Expect(album.OrderAlbumArtistName).To(Equal("OrderAlbumArtistName")) Expect(album.OrderAlbumArtistName).To(Equal("OrderAlbumArtistName"))
+9 -21
View File
@@ -69,27 +69,15 @@ func NewAlbumRepository(ctx context.Context, db dbx.Builder) model.AlbumReposito
"has_rating": hasRatingFilter, "has_rating": hasRatingFilter,
"genre_id": eqFilter, "genre_id": eqFilter,
}) })
if conf.Server.PreferSortTags { r.setSortMappings(map[string]string{
r.sortMappings = map[string]string{ "name": "order_album_name, order_album_artist_name",
"name": "COALESCE(NULLIF(sort_album_name,''),order_album_name)", "artist": "compilation, order_album_artist_name, order_album_name",
"artist": "compilation asc, COALESCE(NULLIF(sort_album_artist_name,''),order_album_artist_name) asc, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc", "album_artist": "compilation, order_album_artist_name, order_album_name",
"album_artist": "compilation asc, COALESCE(NULLIF(sort_album_artist_name,''),order_album_artist_name) asc, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc", "max_year": "coalesce(nullif(original_date,''), cast(max_year as text)), release_date, name",
"max_year": "coalesce(nullif(original_date,''), cast(max_year as text)), release_date, name, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc", "random": "random",
"random": "random", "recently_added": recentlyAddedSort(),
"recently_added": recentlyAddedSort(), "starred_at": "starred, starred_at",
"starred_at": "starred, starred_at", })
}
} else {
r.sortMappings = map[string]string{
"name": "order_album_name asc, order_album_artist_name asc",
"artist": "compilation asc, order_album_artist_name asc, order_album_name asc",
"album_artist": "compilation asc, order_album_artist_name asc, order_album_name asc",
"max_year": "coalesce(nullif(original_date,''), cast(max_year as text)), release_date, name, order_album_name asc",
"random": "random",
"recently_added": recentlyAddedSort(),
"starred_at": "starred, starred_at",
}
}
return r return r
} }
+16 -40
View File
@@ -5,7 +5,7 @@ import (
"context" "context"
"fmt" "fmt"
"net/url" "net/url"
"sort" "slices"
"strings" "strings"
. "github.com/Masterminds/squirrel" . "github.com/Masterminds/squirrel"
@@ -15,7 +15,7 @@ import (
"github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils"
"github.com/navidrome/navidrome/utils/str" "github.com/navidrome/navidrome/utils/slice"
"github.com/pocketbase/dbx" "github.com/pocketbase/dbx"
) )
@@ -67,17 +67,10 @@ func NewArtistRepository(ctx context.Context, db dbx.Builder) model.ArtistReposi
"starred": booleanFilter, "starred": booleanFilter,
"genre_id": eqFilter, "genre_id": eqFilter,
}) })
if conf.Server.PreferSortTags { r.setSortMappings(map[string]string{
r.sortMappings = map[string]string{ "name": "order_artist_name",
"name": "COALESCE(NULLIF(sort_artist_name,''),order_artist_name)", "starred_at": "starred, starred_at",
"starred_at": "starred, starred_at", })
}
} else {
r.sortMappings = map[string]string{
"name": "order_artist_name",
"starred_at": "starred, starred_at",
}
}
return r return r
} }
@@ -143,15 +136,14 @@ func (r *artistRepository) toModels(dba []dbArtist) model.Artists {
return res return res
} }
func (r *artistRepository) getIndexKey(a *model.Artist) string { func (r *artistRepository) getIndexKey(a model.Artist) string {
source := a.Name source := a.OrderArtistName
if conf.Server.PreferSortTags { if conf.Server.PreferSortTags {
source = cmp.Or(a.SortArtistName, a.OrderArtistName, source) source = cmp.Or(a.SortArtistName, a.OrderArtistName)
} }
name := strings.ToLower(str.RemoveArticle(source)) name := strings.ToLower(source)
for k, v := range r.indexGroups { for k, v := range r.indexGroups {
key := strings.ToLower(k) if strings.HasPrefix(name, strings.ToLower(k)) {
if strings.HasPrefix(name, key) {
return v return v
} }
} }
@@ -160,32 +152,16 @@ func (r *artistRepository) getIndexKey(a *model.Artist) string {
// TODO Cache the index (recalculate when there are changes to the DB) // TODO Cache the index (recalculate when there are changes to the DB)
func (r *artistRepository) GetIndex() (model.ArtistIndexes, error) { func (r *artistRepository) GetIndex() (model.ArtistIndexes, error) {
sortColumn := "order_artist_name" artists, err := r.GetAll(model.QueryOptions{Sort: "name"})
if conf.Server.PreferSortTags {
sortColumn = "sort_artist_name, order_artist_name"
}
all, err := r.GetAll(model.QueryOptions{Sort: sortColumn})
if err != nil { if err != nil {
return nil, err return nil, err
} }
fullIdx := make(map[string]*model.ArtistIndex)
for i := range all {
a := all[i]
ax := r.getIndexKey(&a)
idx, ok := fullIdx[ax]
if !ok {
idx = &model.ArtistIndex{ID: ax}
fullIdx[ax] = idx
}
idx.Artists = append(idx.Artists, a)
}
var result model.ArtistIndexes var result model.ArtistIndexes
for _, idx := range fullIdx { for k, v := range slice.Group(artists, r.getIndexKey) {
result = append(result, *idx) result = append(result, model.ArtistIndex{ID: k, Artists: v})
} }
sort.Slice(result, func(i, j int) bool { slices.SortFunc(result, func(a, b model.ArtistIndex) int {
return result[i].ID < result[j].ID return cmp.Compare(a.ID, b.ID)
}) })
return result, nil return result, nil
} }
+122 -138
View File
@@ -5,6 +5,7 @@ import (
"github.com/fatih/structs" "github.com/fatih/structs"
"github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/db"
"github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model"
@@ -46,163 +47,146 @@ var _ = Describe("ArtistRepository", func() {
}) })
Describe("GetIndexKey", func() { Describe("GetIndexKey", func() {
// Note: OrderArtistName should never be empty, so we don't need to test for that
r := artistRepository{indexGroups: utils.ParseIndexGroups(conf.Server.IndexGroups)} r := artistRepository{indexGroups: utils.ParseIndexGroups(conf.Server.IndexGroups)}
It("returns the index key when PreferSortTags is true and SortArtistName is not empty", func() { When("PreferSortTags is false", func() {
conf.Server.PreferSortTags = true BeforeEach(func() {
a := model.Artist{SortArtistName: "Foo", OrderArtistName: "Bar", Name: "Qux"} DeferCleanup(configtest.SetupConfig)
idx := GetIndexKey(&r, &a) // defines export_test.go conf.Server.PreferSortTags = false
Expect(idx).To(Equal("F")) })
It("returns the OrderArtistName key is SortArtistName is empty", func() {
a = model.Artist{SortArtistName: "foo", OrderArtistName: "Bar", Name: "Qux"} conf.Server.PreferSortTags = false
idx = GetIndexKey(&r, &a) a := model.Artist{SortArtistName: "", OrderArtistName: "Bar", Name: "Qux"}
Expect(idx).To(Equal("F")) idx := GetIndexKey(&r, a)
Expect(idx).To(Equal("B"))
})
It("returns the OrderArtistName key even if SortArtistName is not empty", func() {
a := model.Artist{SortArtistName: "Foo", OrderArtistName: "Bar", Name: "Qux"}
idx := GetIndexKey(&r, a)
Expect(idx).To(Equal("B"))
})
}) })
When("PreferSortTags is true", func() {
It("returns the index key when PreferSortTags is true, SortArtistName is empty and OrderArtistName is not empty", func() { BeforeEach(func() {
conf.Server.PreferSortTags = true DeferCleanup(configtest.SetupConfig)
a := model.Artist{SortArtistName: "", OrderArtistName: "Bar", Name: "Qux"} conf.Server.PreferSortTags = true
idx := GetIndexKey(&r, &a) })
Expect(idx).To(Equal("B")) It("returns the SortArtistName key if it is not empty", func() {
a := model.Artist{SortArtistName: "Foo", OrderArtistName: "Bar", Name: "Qux"}
a = model.Artist{SortArtistName: "", OrderArtistName: "bar", Name: "Qux"} idx := GetIndexKey(&r, a)
idx = GetIndexKey(&r, &a) Expect(idx).To(Equal("F"))
Expect(idx).To(Equal("B")) })
}) It("returns the OrderArtistName key if SortArtistName is empty", func() {
a := model.Artist{SortArtistName: "", OrderArtistName: "Bar", Name: "Qux"}
It("returns the index key when PreferSortTags is true, both SortArtistName, OrderArtistName are empty", func() { idx := GetIndexKey(&r, a)
conf.Server.PreferSortTags = true Expect(idx).To(Equal("B"))
a := model.Artist{SortArtistName: "", OrderArtistName: "", Name: "Qux"} })
idx := GetIndexKey(&r, &a)
Expect(idx).To(Equal("Q"))
a = model.Artist{SortArtistName: "", OrderArtistName: "", Name: "qux"}
idx = GetIndexKey(&r, &a)
Expect(idx).To(Equal("Q"))
})
It("returns the index key when PreferSortTags is false and SortArtistName is not empty", func() {
conf.Server.PreferSortTags = false
a := model.Artist{SortArtistName: "Foo", OrderArtistName: "Bar", Name: "Qux"}
idx := GetIndexKey(&r, &a)
Expect(idx).To(Equal("Q"))
})
It("returns the index key when PreferSortTags is true, SortArtistName is empty and OrderArtistName is not empty", func() {
conf.Server.PreferSortTags = false
a := model.Artist{SortArtistName: "", OrderArtistName: "Bar", Name: "Qux"}
idx := GetIndexKey(&r, &a)
Expect(idx).To(Equal("Q"))
})
It("returns the index key when PreferSortTags is true, both sort_artist_name, order_artist_name are empty", func() {
conf.Server.PreferSortTags = false
a := model.Artist{SortArtistName: "", OrderArtistName: "", Name: "Qux"}
idx := GetIndexKey(&r, &a)
Expect(idx).To(Equal("Q"))
a = model.Artist{SortArtistName: "", OrderArtistName: "", Name: "qux"}
idx = GetIndexKey(&r, &a)
Expect(idx).To(Equal("Q"))
}) })
}) })
Describe("GetIndex", func() { Describe("GetIndex", func() {
It("returns the index when PreferSortTags is true and SortArtistName is not empty", func() { When("PreferSortTags is true", func() {
conf.Server.PreferSortTags = true BeforeEach(func() {
DeferCleanup(configtest.SetupConfig)
conf.Server.PreferSortTags = true
})
It("returns the index when SortArtistName is not empty", func() {
artistBeatles.SortArtistName = "Foo"
er := repo.Put(&artistBeatles)
Expect(er).To(BeNil())
artistBeatles.SortArtistName = "Foo" idx, err := repo.GetIndex()
er := repo.Put(&artistBeatles) Expect(err).To(BeNil())
Expect(er).To(BeNil()) Expect(idx).To(Equal(model.ArtistIndexes{
{
idx, err := repo.GetIndex() ID: "F",
Expect(err).To(BeNil()) Artists: model.Artists{
Expect(idx).To(Equal(model.ArtistIndexes{ artistBeatles,
{ },
ID: "F",
Artists: model.Artists{
artistBeatles,
}, },
}, {
{ ID: "K",
ID: "K", Artists: model.Artists{
Artists: model.Artists{ artistKraftwerk,
artistKraftwerk, },
}, },
}, }))
}))
artistBeatles.SortArtistName = "" artistBeatles.SortArtistName = ""
er = repo.Put(&artistBeatles) er = repo.Put(&artistBeatles)
Expect(er).To(BeNil()) Expect(er).To(BeNil())
})
It("returns the index when SortArtistName is empty", func() {
idx, err := repo.GetIndex()
Expect(err).To(BeNil())
Expect(idx).To(Equal(model.ArtistIndexes{
{
ID: "B",
Artists: model.Artists{
artistBeatles,
},
},
{
ID: "K",
Artists: model.Artists{
artistKraftwerk,
},
},
}))
})
}) })
It("returns the index when PreferSortTags is true and SortArtistName is empty", func() { When("PreferSortTags is false", func() {
conf.Server.PreferSortTags = true BeforeEach(func() {
idx, err := repo.GetIndex() DeferCleanup(configtest.SetupConfig)
Expect(err).To(BeNil()) conf.Server.PreferSortTags = false
Expect(idx).To(Equal(model.ArtistIndexes{ })
{ It("returns the index when SortArtistName is not empty", func() {
ID: "B", artistBeatles.SortArtistName = "Foo"
Artists: model.Artists{ er := repo.Put(&artistBeatles)
artistBeatles, Expect(er).To(BeNil())
},
},
{
ID: "K",
Artists: model.Artists{
artistKraftwerk,
},
},
}))
})
It("returns the index when PreferSortTags is false and SortArtistName is not empty", func() { idx, err := repo.GetIndex()
conf.Server.PreferSortTags = false Expect(err).To(BeNil())
Expect(idx).To(Equal(model.ArtistIndexes{
artistBeatles.SortArtistName = "Foo" {
er := repo.Put(&artistBeatles) ID: "B",
Expect(er).To(BeNil()) Artists: model.Artists{
artistBeatles,
idx, err := repo.GetIndex() },
Expect(err).To(BeNil())
Expect(idx).To(Equal(model.ArtistIndexes{
{
ID: "B",
Artists: model.Artists{
artistBeatles,
}, },
}, {
{ ID: "K",
ID: "K", Artists: model.Artists{
Artists: model.Artists{ artistKraftwerk,
artistKraftwerk, },
}, },
}, }))
}))
artistBeatles.SortArtistName = "" artistBeatles.SortArtistName = ""
er = repo.Put(&artistBeatles) er = repo.Put(&artistBeatles)
Expect(er).To(BeNil()) Expect(er).To(BeNil())
}) })
It("returns the index when PreferSortTags is false and SortArtistName is empty", func() { It("returns the index when SortArtistName is empty", func() {
conf.Server.PreferSortTags = false idx, err := repo.GetIndex()
idx, err := repo.GetIndex() Expect(err).To(BeNil())
Expect(err).To(BeNil()) Expect(idx).To(Equal(model.ArtistIndexes{
Expect(idx).To(Equal(model.ArtistIndexes{ {
{ ID: "B",
ID: "B", Artists: model.Artists{
Artists: model.Artists{ artistBeatles,
artistBeatles, },
}, },
}, {
{ ID: "K",
ID: "K", Artists: model.Artists{
Artists: model.Artists{ artistKraftwerk,
artistKraftwerk, },
}, },
}, }))
})) })
}) })
}) })
+122
View File
@@ -0,0 +1,122 @@
package persistence
import (
"database/sql"
"errors"
"fmt"
"regexp"
"github.com/navidrome/navidrome/db"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
// When creating migrations that change existing columns, it is easy to miss the original collation of a column.
// These tests enforce that the required collation of the columns and indexes in the database are kept in place.
// This is important to ensure that the database can perform fast case-insensitive searches and sorts.
var _ = Describe("Collation", func() {
conn := db.Db().ReadDB()
DescribeTable("Column collation",
func(table, column string) {
Expect(checkCollation(conn, table, column)).To(Succeed())
},
Entry("artist.order_artist_name", "artist", "order_artist_name"),
Entry("artist.sort_artist_name", "artist", "sort_artist_name"),
Entry("album.order_album_name", "album", "order_album_name"),
Entry("album.order_album_artist_name", "album", "order_album_artist_name"),
Entry("album.sort_album_name", "album", "sort_album_name"),
Entry("album.sort_album_artist_name", "album", "sort_album_artist_name"),
Entry("media_file.order_title", "media_file", "order_title"),
Entry("media_file.order_album_name", "media_file", "order_album_name"),
Entry("media_file.order_artist_name", "media_file", "order_artist_name"),
Entry("media_file.sort_title", "media_file", "sort_title"),
Entry("media_file.sort_album_name", "media_file", "sort_album_name"),
Entry("media_file.sort_artist_name", "media_file", "sort_artist_name"),
Entry("radio.name", "radio", "name"),
Entry("user.name", "user", "name"),
)
DescribeTable("Index collation",
func(table, column string) {
Expect(checkIndexUsage(conn, table, column)).To(Succeed())
},
Entry("artist.order_artist_name", "artist", "order_artist_name collate nocase"),
Entry("artist.sort_artist_name", "artist", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate nocase"),
Entry("album.order_album_name", "album", "order_album_name collate nocase"),
Entry("album.order_album_artist_name", "album", "order_album_artist_name collate nocase"),
Entry("album.sort_album_name", "album", "coalesce(nullif(sort_album_name,''),order_album_name) collate nocase"),
Entry("album.sort_album_artist_name", "album", "coalesce(nullif(sort_album_artist_name,''),order_album_artist_name) collate nocase"),
Entry("media_file.order_title", "media_file", "order_title collate nocase"),
Entry("media_file.order_album_name", "media_file", "order_album_name collate nocase"),
Entry("media_file.order_artist_name", "media_file", "order_artist_name collate nocase"),
Entry("media_file.sort_title", "media_file", "coalesce(nullif(sort_title,''),order_title) collate nocase"),
Entry("media_file.sort_album_name", "media_file", "coalesce(nullif(sort_album_name,''),order_album_name) collate nocase"),
Entry("media_file.sort_artist_name", "media_file", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate nocase"),
Entry("media_file.path", "media_file", "path collate nocase"),
Entry("radio.name", "radio", "name collate nocase"),
Entry("user.user_name", "user", "user_name collate nocase"),
)
})
func checkIndexUsage(conn *sql.DB, table string, column string) error {
rows, err := conn.Query(fmt.Sprintf(`
explain query plan select * from %[1]s
where %[2]s = 'test'
order by %[2]s`, table, column))
if err != nil {
return err
}
defer rows.Close()
err = rows.Err()
if err != nil {
return err
}
if rows.Next() {
var dummy int
var detail string
err = rows.Scan(&dummy, &dummy, &dummy, &detail)
if err != nil {
return nil
}
if ok, _ := regexp.MatchString("SEARCH.*USING INDEX", detail); ok {
return nil
} else {
return fmt.Errorf("INDEX for '%s' not used: %s", column, detail)
}
}
return errors.New("no rows returned")
}
func checkCollation(conn *sql.DB, table string, column string) error {
rows, err := conn.Query(fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'", table))
if err != nil {
return err
}
defer rows.Close()
err = rows.Err()
if err != nil {
return err
}
if rows.Next() {
var res string
err = rows.Scan(&res)
if err != nil {
return err
}
re := regexp.MustCompile(fmt.Sprintf(`(?i)\b%s\b.*varchar`, column))
if !re.MatchString(res) {
return fmt.Errorf("column '%s' not found in table '%s'", column, table)
}
re = regexp.MustCompile(fmt.Sprintf(`(?i)\b%s\b.*collate\s+NOCASE`, column))
if re.MatchString(res) {
return nil
}
} else {
return fmt.Errorf("table '%s' not found", table)
}
return fmt.Errorf("column '%s' in table '%s' does not have NOCASE collation", column, table)
}
+10
View File
@@ -81,3 +81,13 @@ func (e existsCond) ToSql() (string, []interface{}, error) {
} }
return sql, args, err return sql, args, err
} }
var sortOrderRegex = regexp.MustCompile(`order_([a-z_]+)`)
// Convert the order_* columns to an expression using sort_* columns. Example:
// sort_album_name -> (coalesce(nullif(sort_album_name,”),order_album_name) collate nocase)
// It finds order column names anywhere in the substring
func mapSortOrder(order string) string {
order = strings.ToLower(order)
return sortOrderRegex.ReplaceAllString(order, "(coalesce(nullif(sort_$1,''),order_$1) collate nocase)")
}
+19
View File
@@ -83,4 +83,23 @@ var _ = Describe("Helpers", func() {
Expect(err).To(BeNil()) Expect(err).To(BeNil())
}) })
}) })
Describe("mapSortOrder", func() {
It("does not change the sort string if there are no order columns", func() {
sort := "album_name asc"
mapped := mapSortOrder(sort)
Expect(mapped).To(Equal(sort))
})
It("changes order columns to sort expression", func() {
sort := "ORDER_ALBUM_NAME asc"
mapped := mapSortOrder(sort)
Expect(mapped).To(Equal("(coalesce(nullif(sort_album_name,''),order_album_name) collate nocase) asc"))
})
It("changes multiple order columns to sort expressions", func() {
sort := "compilation, order_title asc, order_album_artist_name desc, year desc"
mapped := mapSortOrder(sort)
Expect(mapped).To(Equal(`compilation, (coalesce(nullif(sort_title,''),order_title) collate nocase) asc,` +
` (coalesce(nullif(sort_album_artist_name,''),order_album_artist_name) collate nocase) desc, year desc`))
})
})
}) })
+8 -32
View File
@@ -10,7 +10,6 @@ import (
. "github.com/Masterminds/squirrel" . "github.com/Masterminds/squirrel"
"github.com/deluan/rest" "github.com/deluan/rest"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model"
"github.com/pocketbase/dbx" "github.com/pocketbase/dbx"
@@ -31,25 +30,14 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) *mediaFileRepos
"starred": booleanFilter, "starred": booleanFilter,
"genre_id": eqFilter, "genre_id": eqFilter,
}) })
if conf.Server.PreferSortTags { r.setSortMappings(map[string]string{
r.sortMappings = map[string]string{ "title": "order_title",
"title": "COALESCE(NULLIF(sort_title,''),order_title)", "artist": "order_artist_name, order_album_name, release_date, disc_number, track_number",
"artist": "COALESCE(NULLIF(sort_artist_name,''),order_artist_name) asc, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc, release_date asc, disc_number asc, track_number asc", "album": "order_album_name, release_date, disc_number, track_number, order_artist_name, title",
"album": "COALESCE(NULLIF(sort_album_name,''),order_album_name) asc, release_date asc, disc_number asc, track_number asc, COALESCE(NULLIF(sort_artist_name,''),order_artist_name) asc, COALESCE(NULLIF(sort_title,''),title) asc", "random": "random",
"random": "random", "created_at": "media_file.created_at",
"created_at": "media_file.created_at", "starred_at": "starred, starred_at",
"starred_at": "starred, starred_at", })
}
} else {
r.sortMappings = map[string]string{
"title": "order_title",
"artist": "order_artist_name asc, order_album_name asc, release_date asc, disc_number asc, track_number asc",
"album": "order_album_name asc, release_date asc, disc_number asc, track_number asc, order_artist_name asc, title asc",
"random": "random",
"created_at": "media_file.created_at",
"starred_at": "starred, starred_at",
}
}
return r return r
} }
@@ -115,18 +103,6 @@ func (r *mediaFileRepository) GetAll(options ...model.QueryOptions) (model.Media
return res, err return res, err
} }
func (r *mediaFileRepository) FindByPath(path string) (*model.MediaFile, error) {
sel := r.newSelect().Columns("*").Where(Like{"path": path})
var res model.MediaFiles
if err := r.queryAll(sel, &res); err != nil {
return nil, err
}
if len(res) == 0 {
return nil, model.ErrNotFound
}
return &res[0], nil
}
func (r *mediaFileRepository) FindByPaths(paths []string) (model.MediaFiles, error) { func (r *mediaFileRepository) FindByPaths(paths []string) (model.MediaFiles, error) {
sel := r.newSelect().Columns("*").Where(Eq{"path collate nocase": paths}) sel := r.newSelect().Columns("*").Where(Eq{"path collate nocase": paths})
var res model.MediaFiles var res model.MediaFiles
+2 -2
View File
@@ -21,9 +21,9 @@ func NewPlayerRepository(ctx context.Context, db dbx.Builder) model.PlayerReposi
r.registerModel(&model.Player{}, map[string]filterFunc{ r.registerModel(&model.Player{}, map[string]filterFunc{
"name": containsFilter("player.name"), "name": containsFilter("player.name"),
}) })
r.sortMappings = map[string]string{ r.setSortMappings(map[string]string{
"user_name": "username", //TODO rename all user_name and userName to username "user_name": "username", //TODO rename all user_name and userName to username
} })
return r return r
} }
+2 -2
View File
@@ -55,9 +55,9 @@ func NewPlaylistRepository(ctx context.Context, db dbx.Builder) model.PlaylistRe
"q": playlistFilter, "q": playlistFilter,
"smart": smartPlaylistFilter, "smart": smartPlaylistFilter,
}) })
r.sortMappings = map[string]string{ r.setSortMappings(map[string]string{
"owner_name": "owner_name", "owner_name": "owner_name",
} })
return r return r
} }
+4 -10
View File
@@ -5,7 +5,6 @@ import (
. "github.com/Masterminds/squirrel" . "github.com/Masterminds/squirrel"
"github.com/deluan/rest" "github.com/deluan/rest"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/slice" "github.com/navidrome/navidrome/utils/slice"
@@ -26,18 +25,13 @@ func (r *playlistRepository) Tracks(playlistId string, refreshSmartPlaylist bool
p.db = r.db p.db = r.db
p.tableName = "playlist_tracks" p.tableName = "playlist_tracks"
p.registerModel(&model.PlaylistTrack{}, nil) p.registerModel(&model.PlaylistTrack{}, nil)
p.sortMappings = map[string]string{ p.setSortMappings(map[string]string{
"id": "playlist_tracks.id", "id": "playlist_tracks.id",
"artist": "order_artist_name asc", "artist": "order_artist_name",
"album": "order_album_name asc, order_album_artist_name asc", "album": "order_album_name, order_album_artist_name",
"title": "order_title", "title": "order_title",
"duration": "duration", // To make sure the field will be whitelisted "duration": "duration", // To make sure the field will be whitelisted
} })
if conf.Server.PreferSortTags {
p.sortMappings["artist"] = "COALESCE(NULLIF(sort_artist_name,''),order_artist_name) asc"
p.sortMappings["album"] = "COALESCE(NULLIF(sort_album_name,''),order_album_name)"
p.sortMappings["title"] = "COALESCE(NULLIF(sort_title,''),title)"
}
pls, err := r.Get(playlistId) pls, err := r.Get(playlistId)
if err != nil { if err != nil {
-3
View File
@@ -24,9 +24,6 @@ func NewRadioRepository(ctx context.Context, db dbx.Builder) model.RadioReposito
r.registerModel(&model.Radio{}, map[string]filterFunc{ r.registerModel(&model.Radio{}, map[string]filterFunc{
"name": containsFilter("name"), "name": containsFilter("name"),
}) })
r.sortMappings = map[string]string{
"name": "(name collate nocase), name",
}
return r return r
} }
+3 -3
View File
@@ -23,10 +23,10 @@ func NewShareRepository(ctx context.Context, db dbx.Builder) model.ShareReposito
r := &shareRepository{} r := &shareRepository{}
r.ctx = ctx r.ctx = ctx
r.db = db r.db = db
r.registerModel(&model.Share{}, map[string]filterFunc{}) r.registerModel(&model.Share{}, nil)
r.sortMappings = map[string]string{ r.setSortMappings(map[string]string{
"username": "username", "username": "username",
} })
return r return r
} }
+24 -5
View File
@@ -27,17 +27,20 @@ import (
// - Call registerModel with the model instance and any possible filters. // - Call registerModel with the model instance and any possible filters.
// - If the model has a different table name than the default (lowercase of the model name), it should be set manually // - If the model has a different table name than the default (lowercase of the model name), it should be set manually
// using the tableName field. // using the tableName field.
// - Sort mappings should be set in the sortMappings field. If the sort field is not in the map, it will be used as is. // - Sort mappings must be set with setSortMappings method. If a sort field is not in the map, it will be used as the name of the column.
// //
// All fields in filters and sortMappings must be in snake_case. Only sorts and filters based on real field names or // All fields in filters and sortMappings must be in snake_case. Only sorts and filters based on real field names or
// defined in the mappings will be allowed. // defined in the mappings will be allowed.
type sqlRepository struct { type sqlRepository struct {
ctx context.Context ctx context.Context
tableName string tableName string
db dbx.Builder db dbx.Builder
sortMappings map[string]string
// Do not set these fields manually, they are set by the registerModel method
filterMappings map[string]filterFunc filterMappings map[string]filterFunc
isFieldWhiteListed fieldWhiteListedFunc isFieldWhiteListed fieldWhiteListedFunc
// Do not set this field manually, it is set by the setSortMappings method
sortMappings map[string]string
} }
const invalidUserId = "-1" const invalidUserId = "-1"
@@ -68,6 +71,22 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun
r.filterMappings = filters r.filterMappings = filters
} }
// setSortMappings sets the mappings for the sort fields. If the sort field is not in the map, it will be used as is.
//
// If PreferSortTags is enabled, it will map the order fields to the corresponding sort expression,
// which gives precedence to sort tags.
// Ex: order_title => (coalesce(nullif(sort_title,”),order_title) collate nocase)
// To avoid performance issues, indexes should be created for these sort expressions
func (r *sqlRepository) setSortMappings(mappings map[string]string) {
if conf.Server.PreferSortTags {
for k, v := range mappings {
v = mapSortOrder(v)
mappings[k] = v
}
}
r.sortMappings = mappings
}
func (r sqlRepository) getTableName() string { func (r sqlRepository) getTableName() string {
return r.tableName return r.tableName
} }
+7 -10
View File
@@ -2,6 +2,7 @@ package scanner
import ( import (
"context" "context"
"strconv"
"github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/artwork"
@@ -70,18 +71,14 @@ type mockedMediaFile struct {
model.MediaFileRepository model.MediaFileRepository
} }
func (r *mockedMediaFile) FindByPath(s string) (*model.MediaFile, error) {
return &model.MediaFile{
ID: "123",
Path: s,
}, nil
}
func (r *mockedMediaFile) FindByPaths(paths []string) (model.MediaFiles, error) { func (r *mockedMediaFile) FindByPaths(paths []string) (model.MediaFiles, error) {
var mfs model.MediaFiles var mfs model.MediaFiles
for _, path := range paths { for i, path := range paths {
mf, _ := r.FindByPath(path) mf := model.MediaFile{
mfs = append(mfs, *mf) ID: strconv.Itoa(i),
Path: path,
}
mfs = append(mfs, mf)
} }
return mfs, nil return mfs, nil
} }