Fix ambiguous column when sorting media_files by created_at.

Fix #3006
navidrome · May 8, 2024 · 62cc8a2 · 62cc8a2
1 parent dd4374c
commit 62cc8a2
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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() {