From 62cc8a2d4bc1b901e81c6f35650eef4a8a4c7333 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 8 May 2024 08:22:53 -0400 Subject: [PATCH] Fix ambiguous column when sorting media_files by created_at. Fix #3006 --- persistence/helpers.go | 8 ++++++ persistence/helpers_test.go | 14 +++++++++ persistence/mediafile_repository.go | 16 ++++++----- persistence/sql_base_repository.go | 17 +++++++++-- persistence/sql_base_repository_test.go | 38 +++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 10 deletions(-) diff --git a/persistence/helpers.go b/persistence/helpers.go index 4a62955f3..e6edf61d6 100644 --- a/persistence/helpers.go +++ b/persistence/helpers.go @@ -51,6 +51,14 @@ func toSnakeCase(str string) string { return strings.ToLower(snake) } +var matchUnderscore = regexp.MustCompile("_([A-Za-z])") + +func toCamelCase(str string) string { + return matchUnderscore.ReplaceAllStringFunc(str, func(s string) string { + return strings.ToUpper(strings.Replace(s, "_", "", -1)) + }) +} + func exists(subTable string, cond squirrel.Sqlizer) existsCond { return existsCond{subTable: subTable, cond: cond, not: false} } diff --git a/persistence/helpers_test.go b/persistence/helpers_test.go index f6060825d..f080d1e81 100644 --- a/persistence/helpers_test.go +++ b/persistence/helpers_test.go @@ -23,6 +23,20 @@ var _ = Describe("Helpers", func() { Expect(toSnakeCase("snake_case")).To(Equal("snake_case")) }) }) + Describe("toCamelCase", func() { + It("converts snake_case", func() { + Expect(toCamelCase("snake_case")).To(Equal("snakeCase")) + }) + It("converts PascalCase", func() { + Expect(toCamelCase("PascalCase")).To(Equal("PascalCase")) + }) + It("converts camelCase", func() { + Expect(toCamelCase("camelCase")).To(Equal("camelCase")) + }) + It("converts ALLCAPS", func() { + Expect(toCamelCase("ALLCAPS")).To(Equal("ALLCAPS")) + }) + }) Describe("toSQLArgs", func() { type Embed struct{} type Model struct { diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index 139ea65b1..9766e57d3 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -33,16 +33,18 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) *mediaFileRepos } if conf.Server.PreferSortTags { r.sortMappings = map[string]string{ - "title": "COALESCE(NULLIF(sort_title,''),title)", - "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": "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()", + "title": "COALESCE(NULLIF(sort_title,''),title)", + "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": "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()", + "createdAt": "media_file.created_at", } } else { r.sortMappings = map[string]string{ - "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()", + "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()", + "createdAt": "media_file.created_at", } } return r diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index 3bb050e89..d5282516d 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -68,12 +68,23 @@ func (r sqlRepository) applyOptions(sq SelectBuilder, options ...model.QueryOpti return sq } -func (r sqlRepository) buildSortOrder(sort, order string) string { +// TODO Change all sortMappings to have a consistent case +func (r sqlRepository) sortMapping(sort string) string { if mapping, ok := r.sortMappings[sort]; ok { - sort = mapping + return mapping + } + if mapping, ok := r.sortMappings[toCamelCase(sort)]; ok { + return mapping } - sort = toSnakeCase(sort) + if mapping, ok := r.sortMappings[sort]; ok { + return mapping + } + return sort +} + +func (r sqlRepository) buildSortOrder(sort, order string) string { + sort = r.sortMapping(sort) order = strings.ToLower(strings.TrimSpace(order)) var reverseOrder string if order == "desc" { diff --git a/persistence/sql_base_repository_test.go b/persistence/sql_base_repository_test.go index 5e108192d..83fad4b39 100644 --- a/persistence/sql_base_repository_test.go +++ b/persistence/sql_base_repository_test.go @@ -70,6 +70,44 @@ var _ = Describe("sqlRepository", func() { }) }) + Describe("sortMapping", func() { + BeforeEach(func() { + r.sortMappings = map[string]string{ + "sort1": "mappedSort1", + "sortTwo": "mappedSort2", + "sort_three": "mappedSort3", + } + }) + + It("returns the mapped value when sort key exists", func() { + Expect(r.sortMapping("sort1")).To(Equal("mappedSort1")) + }) + + Context("when sort key does not exist", func() { + It("returns the original sort key, snake cased", func() { + Expect(r.sortMapping("NotFoundSort")).To(Equal("not_found_sort")) + }) + }) + + Context("when sort key is camel cased", func() { + It("returns the mapped value when camel case sort key exists", func() { + Expect(r.sortMapping("sortTwo")).To(Equal("mappedSort2")) + }) + It("returns the mapped value when passing a snake case key", func() { + Expect(r.sortMapping("sort_two")).To(Equal("mappedSort2")) + }) + }) + + Context("when sort key is snake cased", func() { + It("returns the mapped value when snake case sort key exists", func() { + Expect(r.sortMapping("sort_three")).To(Equal("mappedSort3")) + }) + It("returns the mapped value when passing a camel case key", func() { + Expect(r.sortMapping("sortThree")).To(Equal("mappedSort3")) + }) + }) + }) + Describe("buildSortOrder", func() { Context("single field", func() { It("sorts by specified field", func() {