fix: address third code review findings
- Prevent infinite pagination loop when API returns empty release groups page - Move NormalizeString regexes to package level to avoid recompilation on every call - Use UTC consistently for cached_at timestamps to avoid DST-related TTL skew
This commit is contained in:
@@ -26,6 +26,14 @@ var includedTypes = map[string]bool{
|
||||
"Compilation": true,
|
||||
}
|
||||
|
||||
// Precompiled regexes for NormalizeString — compiled once at package init.
|
||||
var (
|
||||
bracketRe = regexp.MustCompile(`\[[^\]]*\]`)
|
||||
parenRe = regexp.MustCompile(`\([^)]*\)`)
|
||||
yearRe = regexp.MustCompile(`\b(1[0-9]{3}|2[0-9]{3})\b`)
|
||||
spaceRe = regexp.MustCompile(`\s+`)
|
||||
)
|
||||
|
||||
// GetArtistReleaseGroups fetches all release groups for a given artist from MusicBrainz.
|
||||
// It queries the artist's release groups via the MusicBrainz Web Service API,
|
||||
// parses the XML response, and applies status and type filtering.
|
||||
@@ -57,7 +65,9 @@ func (c *MusicBrainzClient) GetArtistReleaseGroups(ctx context.Context, artistMB
|
||||
allGroups = append(allGroups, parsed.ReleaseGroups...)
|
||||
|
||||
// If we've fetched all results, we've reached the end.
|
||||
if offset+len(parsed.ReleaseGroups) >= parsed.Count {
|
||||
// Also break on empty page to prevent infinite loop if API
|
||||
// returns fewer items than advertised by count.
|
||||
if len(parsed.ReleaseGroups) == 0 || offset+len(parsed.ReleaseGroups) >= parsed.Count {
|
||||
break
|
||||
}
|
||||
|
||||
@@ -111,15 +121,12 @@ func NormalizeString(s string) string {
|
||||
s = strings.ToLower(s)
|
||||
|
||||
// Remove bracketed content first (e.g., [Deluxe Edition], [Remastered 2020])
|
||||
bracketRe := regexp.MustCompile(`\[[^\]]*\]`)
|
||||
s = bracketRe.ReplaceAllString(s, "")
|
||||
|
||||
// Remove parenthesized content (e.g., (Deluxe), (Remastered))
|
||||
parenRe := regexp.MustCompile(`\([^)]*\)`)
|
||||
s = parenRe.ReplaceAllString(s, "")
|
||||
|
||||
// Remove years (4-digit numbers between 1000-2999)
|
||||
yearRe := regexp.MustCompile(`\b(1[0-9]{3}|2[0-9]{3})\b`)
|
||||
s = yearRe.ReplaceAllString(s, "")
|
||||
|
||||
// Replace common separators with spaces before stripping other special chars
|
||||
@@ -136,7 +143,6 @@ func NormalizeString(s string) string {
|
||||
s = b.String()
|
||||
|
||||
// Collapse multiple spaces
|
||||
spaceRe := regexp.MustCompile(`\s+`)
|
||||
s = spaceRe.ReplaceAllString(s, " ")
|
||||
|
||||
// Trim
|
||||
|
||||
Reference in New Issue
Block a user