Skip to content

Commit

Permalink
fix: linting
Browse files Browse the repository at this point in the history
fix: unit tests
change: make `getResponse` private again
add: implement `addArtistToQueue`, which is currently unused
  • Loading branch information
xxxserxxx committed Dec 23, 2024
1 parent da06512 commit 4eabb99
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ All notable changes to this project will be documented in this file.
- Browser now works on metadata, not directory structure. Not all functions fully tested, but it works.
- Tweaks the lyrics timing offset to try to hit that sweet spot that makes up for the inaccuracy of mpv
- Rolls back the (unrelated, unnecessary) logger changes
- Changelog is updated, and readme updated to list the extra merged features

## [0.1.0] - 2024-10-15

Expand Down
1 change: 1 addition & 0 deletions event_loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func (ui *Ui) guiEventLoop() {
// in the future
p := statusData.Position*1000 + 500
_, _, _, fh := ui.queuePage.lyrics.GetInnerRect()
// FIXME the lyrics lookup would perform better as a binary search
for i := 0; i < lcl-1; i++ {
if p >= cl[i].Start && p < cl[i+1].Start {
txt := ""
Expand Down
3 changes: 3 additions & 0 deletions helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ const (
)

// if the first argument isn't empty, return it, otherwise return the second
//
//nolint:golint,unused
func stringOr(firstChoice string, secondChoice string) string {
// TODO stringOr is not used anymore, so commented out until removal.
if firstChoice != "" {
return firstChoice
}
Expand Down
37 changes: 33 additions & 4 deletions page_browser.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ func (ui *Ui) createBrowserPage(artists []subsonic.Artist) *BrowserPage {
}
artists := make([]subsonic.Artist, 0)
for _, ind := range artistsIndex.Index {
for _, art := range ind.Artists {
artists = append(artists, art)
}
artists = append(artists, ind.Artists...)
}
sort.Slice(artists, func(i, j int) bool {
return artists[i].Name < artists[j].Name
Expand Down Expand Up @@ -465,7 +463,37 @@ func entityListTextFormat(id, title string, dir bool, starredItems map[string]st
return tview.Escape(title) + star
}

func (b *BrowserPage) addArtistToQueue(artist subsonic.Artist) {}
//nolint:golint,unused
func (b *BrowserPage) addArtistToQueue(artist subsonic.Artist) {
var err error
// If the artist is sparse, populate it
if len(artist.Albums) == 0 {
artist, err = b.ui.connection.GetArtist(artist.Id)
if err != nil {
b.logger.Printf("addArtistToQueue: GetArtist %s -- %s", artist.Id, err.Error())
return
}
// If it's _still_ sparse, return, as there's nothing to do
if len(artist.Albums) == 0 {
b.logger.Printf("addArtistToQueue: artist %q (%q) has no albums", artist.Name, artist.Id)
return
}
}

for _, album := range artist.Albums {
if len(album.Songs) == 0 {
album, err = b.ui.connection.GetAlbum(album.Id)
if err != nil {
b.logger.Printf("addArtistToQueue: GetAlbum %s -- %s", album.Id, err.Error())
// Hope the error is transient
continue
}
}
for _, s := range album.Songs {
b.ui.addSongToQueue(s)
}
}
}

func (b *BrowserPage) addAlbumToQueue(album subsonic.Album) {
var err error
Expand All @@ -483,6 +511,7 @@ func (b *BrowserPage) addAlbumToQueue(album subsonic.Album) {
}
}

//nolint:golint,unused
func (b *BrowserPage) addDirectoryToQueue(entity *subsonic.Entity) {
directory, err := b.ui.connection.GetMusicDirectory(entity.Id)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions subsonic/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func TestGetResponse(t *testing.T) {
if err != nil {
t.Errorf("expected no error but got: %v", err)
}

if response == nil {
t.Errorf("expected a response but got nil")
}
Expand Down
62 changes: 31 additions & 31 deletions subsonic/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,16 @@ func defaultQuery(connection *Connection) url.Values {
func (connection *Connection) GetServerInfo() (Response, error) {
query := defaultQuery(connection)
requestUrl := connection.Host + "/rest/ping" + "?" + query.Encode()
return connection.GetResponse("GetServerInfo", requestUrl)
r, e := connection.getResponse("GetServerInfo", requestUrl)
return *r, e
}

// GetIndexes returns an indexed structure of all artists
// https://opensubsonic.netlify.app/docs/endpoints/getindexes/
func (connection *Connection) GetIndexes() (Indexes, error) {
query := defaultQuery(connection)
requestUrl := connection.Host + "/rest/getIndexes" + "?" + query.Encode()
i, e := connection.GetResponse("GetIndexes", requestUrl)
i, e := connection.getResponse("GetIndexes", requestUrl)
return i.Indexes, e
}

Expand All @@ -117,7 +118,7 @@ func (connection *Connection) GetIndexes() (Indexes, error) {
func (connection *Connection) GetArtists() (Indexes, error) {
query := defaultQuery(connection)
requestUrl := connection.Host + "/rest/getArtists" + "?" + query.Encode()
i, e := connection.GetResponse("GetArtists", requestUrl)
i, e := connection.getResponse("GetArtists", requestUrl)
return i.Artists, e
}

Expand All @@ -134,7 +135,7 @@ func (connection *Connection) GetArtist(id string) (Artist, error) {
query := defaultQuery(connection)
query.Set("id", id)
requestUrl := connection.Host + "/rest/getArtist" + "?" + query.Encode()
resp, err := connection.GetResponse("GetArtist", requestUrl)
resp, err := connection.getResponse("GetArtist", requestUrl)
if err != nil {
return resp.Artist, err
}
Expand Down Expand Up @@ -170,7 +171,7 @@ func (connection *Connection) GetAlbum(id string) (Album, error) {
query := defaultQuery(connection)
query.Set("id", id)
requestUrl := connection.Host + "/rest/getAlbum" + "?" + query.Encode()
resp, err := connection.GetResponse("GetAlbum", requestUrl)
resp, err := connection.getResponse("GetAlbum", requestUrl)
if err != nil {
return resp.Album, err
}
Expand Down Expand Up @@ -202,7 +203,7 @@ func (connection *Connection) GetMusicDirectory(id string) (Directory, error) {
query := defaultQuery(connection)
query.Set("id", id)
requestUrl := connection.Host + "/rest/getMusicDirectory" + "?" + query.Encode()
resp, err := connection.GetResponse("GetMusicDirectory", requestUrl)
resp, err := connection.getResponse("GetMusicDirectory", requestUrl)
if err != nil {
return resp.Directory, err
}
Expand Down Expand Up @@ -295,14 +296,14 @@ func (connection *Connection) GetRandomSongs(Id string) (Entities, error) {
if Id == "" {
query.Set("size", size)
requestUrl := connection.Host + "/rest/getRandomSongs?" + query.Encode()
resp, err := connection.GetResponse("GetRandomSongs", requestUrl)
resp, err := connection.getResponse("GetRandomSongs", requestUrl)
return resp.RandomSongs.Songs, err
}

query.Set("id", Id)
query.Set("count", size)
requestUrl := connection.Host + "/rest/getSimilarSongs?" + query.Encode()
resp, err := connection.GetResponse("GetSimilar", requestUrl)
resp, err := connection.getResponse("GetSimilar", requestUrl)
return resp.SimilarSongs.Songs, err
}

Expand All @@ -314,14 +315,14 @@ func (connection *Connection) ScrobbleSubmission(id string, isSubmission bool) (
query.Set("submission", strconv.FormatBool(isSubmission))

requestUrl := connection.Host + "/rest/scrobble" + "?" + query.Encode()
resp, err := connection.GetResponse("ScrobbleSubmission", requestUrl)
return resp, err
resp, err := connection.getResponse("ScrobbleSubmission", requestUrl)
return *resp, err
}

func (connection *Connection) GetStarred() (Results, error) {
query := defaultQuery(connection)
requestUrl := connection.Host + "/rest/getStarred" + "?" + query.Encode()
resp, err := connection.GetResponse("GetStarred", requestUrl)
resp, err := connection.getResponse("GetStarred", requestUrl)
return resp.Starred, err
}

Expand All @@ -337,23 +338,23 @@ func (connection *Connection) ToggleStar(id string, starredItems map[string]stru
}

requestUrl := connection.Host + "/rest/" + action + "?" + query.Encode()
resp, err := connection.GetResponse("ToggleStar", requestUrl)
resp, err := connection.getResponse("ToggleStar", requestUrl)
if err != nil {
if ok {
delete(starredItems, id)
} else {
starredItems[id] = struct{}{}
}
return resp, err
return *resp, err
}
return resp, nil
return *resp, nil
}

// FIXME this diverges from the rest of the code by recursively fetching all the data, which is why all of the background loading code was necessary. Strip all that out, and have playlists load as the user scrolls
func (connection *Connection) GetPlaylists() (Playlists, error) {
query := defaultQuery(connection)
requestUrl := connection.Host + "/rest/getPlaylists" + "?" + query.Encode()
resp, err := connection.GetResponse("GetPlaylists", requestUrl)
resp, err := connection.getResponse("GetPlaylists", requestUrl)
if err != nil {
return resp.Playlists, err
}
Expand Down Expand Up @@ -384,7 +385,7 @@ func (connection *Connection) GetPlaylist(id string) (Playlist, error) {
query.Set("id", id)

requestUrl := connection.Host + "/rest/getPlaylist" + "?" + query.Encode()
resp, err := connection.GetResponse("GetPlaylist", requestUrl)
resp, err := connection.getResponse("GetPlaylist", requestUrl)
return resp.Playlist, err
}

Expand All @@ -409,39 +410,38 @@ func (connection *Connection) CreatePlaylist(id, name string, songIds []string)
query.Add("songId", sid)
}
requestUrl := connection.Host + "/rest/createPlaylist" + "?" + query.Encode()
resp, err := connection.GetResponse("GetPlaylist", requestUrl)
resp, err := connection.getResponse("GetPlaylist", requestUrl)
return resp.Playlist, err
}

func (connection *Connection) GetResponse(caller, requestUrl string) (Response, error) {
zero := Response{}
func (connection *Connection) getResponse(caller, requestUrl string) (*Response, error) {
res, err := http.Get(requestUrl)
if err != nil {
return zero, fmt.Errorf("[%s] failed to make GET request: %v", caller, err)
return nil, fmt.Errorf("[%s] failed to make GET request: %v", caller, err)
}

if res.Body != nil {
defer res.Body.Close()
} else {
return zero, fmt.Errorf("[%s] response body is nil", caller)
return nil, fmt.Errorf("[%s] response body is nil", caller)
}

if res.StatusCode != http.StatusOK {
return zero, fmt.Errorf("[%s] unexpected status code: %d, status: %s", caller, res.StatusCode, res.Status)
return nil, fmt.Errorf("[%s] unexpected status code: %d, status: %s", caller, res.StatusCode, res.Status)
}

responseBody, readErr := io.ReadAll(res.Body)
if readErr != nil {
return zero, fmt.Errorf("[%s] failed to read response body: %v", caller, readErr)
return nil, fmt.Errorf("[%s] failed to read response body: %v", caller, readErr)
}

var decodedBody responseWrapper
err = json.Unmarshal(responseBody, &decodedBody)
if err != nil {
return zero, fmt.Errorf("[%s] failed to unmarshal response body: %v", caller, err)
return nil, fmt.Errorf("[%s] failed to unmarshal response body: %v", caller, err)
}

return decodedBody.Response, nil
return &decodedBody.Response, nil
}

func (connection *Connection) DeletePlaylist(id string) error {
Expand Down Expand Up @@ -494,7 +494,7 @@ func (connection *Connection) Search(searchTerm string, artistOffset, albumOffse
query.Set("albumOffset", strconv.Itoa(albumOffset))
query.Set("songOffset", strconv.Itoa(songOffset))
requestUrl := connection.Host + "/rest/search3" + "?" + query.Encode()
res, err := connection.GetResponse("Search", requestUrl)
res, err := connection.getResponse("Search", requestUrl)
return Results(res.SearchResult3), err
}

Expand All @@ -504,7 +504,7 @@ func (connection *Connection) Search(searchTerm string, artistOffset, albumOffse
func (connection *Connection) StartScan() error {
query := defaultQuery(connection)
requestUrl := fmt.Sprintf("%s/rest/startScan?%s", connection.Host, query.Encode())
if res, err := connection.GetResponse("StartScan", requestUrl); err != nil {
if res, err := connection.getResponse("StartScan", requestUrl); err != nil {
return err
} else if !res.ScanStatus.Scanning {
return fmt.Errorf("server returned false for scan status on scan attempt")
Expand All @@ -520,14 +520,14 @@ func (connection *Connection) SavePlayQueue(queueIds []string, current string, p
query.Set("current", current)
query.Set("position", fmt.Sprintf("%d", position))
requestUrl := fmt.Sprintf("%s/rest/savePlayQueue?%s", connection.Host, query.Encode())
_, err := connection.GetResponse("SavePlayQueue", requestUrl)
_, err := connection.getResponse("SavePlayQueue", requestUrl)
return err
}

func (connection *Connection) LoadPlayQueue() (PlayQueue, error) {
query := defaultQuery(connection)
requestUrl := fmt.Sprintf("%s/rest/getPlayQueue?%s", connection.Host, query.Encode())
resp, err := connection.GetResponse("GetPlayQueue", requestUrl)
resp, err := connection.getResponse("GetPlayQueue", requestUrl)
return resp.PlayQueue, err
}

Expand Down Expand Up @@ -576,7 +576,7 @@ func (connection *Connection) GetLyricsBySongId(id string) ([]StructuredLyrics,
func (connection *Connection) GetGenres() ([]GenreEntry, error) {
query := defaultQuery(connection)
requestUrl := connection.Host + "/rest/getGenres" + "?" + query.Encode()
resp, err := connection.GetResponse("GetGenres", requestUrl)
resp, err := connection.getResponse("GetGenres", requestUrl)
if err != nil {
return resp.Genres.Genres, err
}
Expand All @@ -593,7 +593,7 @@ func (connection *Connection) GetSongsByGenre(genre string, offset int, musicFol
query.Add("musicFolderId", musicFolderID)
}
requestUrl := connection.Host + "/rest/getSongsByGenre" + "?" + query.Encode()
resp, err := connection.GetResponse("GetPlaylists", requestUrl)
resp, err := connection.getResponse("GetPlaylists", requestUrl)
if err != nil {
return resp.SongsByGenre.Songs, err
}
Expand Down
4 changes: 1 addition & 3 deletions subsonic/lru.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ func NewLRU[T any](m map[string]T, size int) LRU[T] {
// Push a key onto the front of the stack. If Push is called repeatedly with the
// same key, it will flush all other items out of the stack.
func (l *LRU[T]) Push(key string) {
if _, ok := l.managedMap[l.ring[l.idx]]; ok {
delete(l.managedMap, l.ring[l.idx])
}
delete(l.managedMap, l.ring[l.idx])
if l.idx < len(l.ring)-1 {
l.idx++
} else {
Expand Down

0 comments on commit 4eabb99

Please sign in to comment.