Expand SoundCloud downloading functionality#820
Conversation
| if !ok && provider == model.ProviderSoundcloud { | ||
| val, ok = os.LookupEnv("PODSYNC_SOUNDCLOUD_API_KEY") | ||
| if ok { | ||
| envVar = "PODSYNC_SOUNDCLOUD_API_KEY" |
There was a problem hiding this comment.
Could you pls output warning? That using deprecated key which will be removed in future versions
There was a problem hiding this comment.
Pull request overview
This PR expands SoundCloud support in Podsync to allow fetching feeds from SoundCloud user profiles (uploads) in addition to the previously supported playlist URLs. It refactors the SoundCloud URL parser, builder, and configuration to handle both URL types, makes the SoundCloud API key optional (since the underlying library can auto-scrape a client_id), and adds backward compatibility for the renamed environment variable.
Changes:
- Extended
parseSoundcloudURLto recognize user profile URLs (/username,/username/tracks) alongside playlist URLs (/username/sets/<playlist>), with reserved route filtering - Added
buildUsermethod to the SoundCloud builder that resolves user profiles and fetches their tracks via the SoundCloud API v2, plus refactored existing playlist logic intobuildPlaylistwith a sharedtrackToEpisodehelper - Made SoundCloud API key optional throughout the stack (updater, config, builder factory) and renamed the environment variable from
PODSYNC_SOUNDCLOUD_API_KEYtoPODSYNC_SOUNDCLOUD_CLIENT_IDwith backward compatibility
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/builder/url.go |
Rewrote parseSoundcloudURL to support user profile and playlist URLs with reserved route filtering |
pkg/builder/url_test.go |
Added unit tests for new SoundCloud URL parsing (playlist, user profile, invalid URLs) |
pkg/builder/soundcloud.go |
Refactored builder into buildPlaylist/buildUser methods, added trackToEpisode helper and fetchUserTracks for API v2 integration |
pkg/builder/soundcloud_test.go |
Updated integration tests to cover user profile feeds and relax assertions for optional fields |
pkg/builder/builder.go |
Passes optional key parameter to NewSoundcloudBuilder |
services/update/updater.go |
Allows missing key provider for SoundCloud |
cmd/podsync/config.go |
Renamed env var to PODSYNC_SOUNDCLOUD_CLIENT_ID with backward compat for PODSYNC_SOUNDCLOUD_API_KEY |
config.toml.example |
Documents optional SoundCloud client_id config |
README.md |
Updated features list, added SoundCloud URL format docs, updated env var table, but regressed Docker run formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var tracks []soundcloudapi.Track | ||
| if err := json.NewDecoder(resp.Body).Decode(&tracks); err != nil { | ||
| return nil, errors.Wrap(err, "failed to decode soundcloud user tracks response") | ||
| } | ||
|
|
||
| return tracks, nil |
There was a problem hiding this comment.
The SoundCloud API v2 /users/{id}/tracks endpoint returns a paginated JSON object with a collection key containing the track array (e.g., {"collection": [...], "next_href": "..."}), not a bare JSON array. Decoding directly into []soundcloudapi.Track will fail with a JSON unmarshalling error at runtime.
You need to define a wrapper struct (e.g., struct { Collection []soundcloudapi.Track \json:"collection"` }) and decode into that, then extract the .Collection` field.
| var tracks []soundcloudapi.Track | |
| if err := json.NewDecoder(resp.Body).Decode(&tracks); err != nil { | |
| return nil, errors.Wrap(err, "failed to decode soundcloud user tracks response") | |
| } | |
| return tracks, nil | |
| var apiResp struct { | |
| Collection []soundcloudapi.Track `json:"collection"` | |
| } | |
| if err := json.NewDecoder(resp.Body).Decode(&apiResp); err != nil { | |
| return nil, errors.Wrap(err, "failed to decode soundcloud user tracks response") | |
| } | |
| return apiResp.Collection, nil |
| -v $(pwd)/db:/app/db/ \ | ||
| -v $(pwd)/config.toml:/app/config.toml \ | ||
| ghcr.io/mxpv/podsync:latest | ||
| $ docker run -p 8080:8080 -v $(pwd)/data:/app/data/ -v $(pwd)/db:/app/db/ -v $(pwd)/config.toml:/app/config.toml ghcr.io/mxpv/podsync:latest |
There was a problem hiding this comment.
The Docker run command has been collapsed from a multi-line \-continued command into a single long line, making it much harder to read in the documentation. This appears to be an unintentional formatting regression. It should be kept as a multi-line command with backslash continuation for readability.
| $ docker run -p 8080:8080 -v $(pwd)/data:/app/data/ -v $(pwd)/db:/app/db/ -v $(pwd)/config.toml:/app/config.toml ghcr.io/mxpv/podsync:latest | |
| $ docker run \ | |
| -p 8080:8080 \ | |
| -v $(pwd)/data:/app/data/ \ | |
| -v $(pwd)/db:/app/db/ \ | |
| -v $(pwd)/config.toml:/app/config.toml \ | |
| ghcr.io/mxpv/podsync:latest |
| user, err := s.client.GetUser(soundcloudapi.GetUserOptions{ | ||
| ProfileURL: cfg.URL, | ||
| }) |
There was a problem hiding this comment.
When the user provides https://soundcloud.com/username/tracks, cfg.URL is passed directly to s.client.GetUser(soundcloudapi.GetUserOptions{ProfileURL: cfg.URL}). The SoundCloud resolve API may not recognize https://soundcloud.com/username/tracks as a valid profile URL (only https://soundcloud.com/username resolves to a user). You should strip the /tracks suffix before passing the URL to GetUser, or reconstruct the profile URL from the parsed username (e.g., "https://soundcloud.com/" + info.ItemID).
| limit := cfg.PageSize | ||
| if limit <= 0 { | ||
| // Keep a sane default; the feed can still be "unlimited" by setting PageSize high. | ||
| limit = 20 |
There was a problem hiding this comment.
The hardcoded default of 20 is inconsistent with the project's DefaultPageSize of 50 (defined in pkg/model/defaults.go:10). Moreover, PageSize will never actually be <= 0 at this point because config.go:179-181 sets it to model.DefaultPageSize (50) if it's 0. This means the limit <= 0 branch is dead code, and the fallback to 20 would never trigger. Consider removing this dead code, or if you want to keep a safety check, use model.DefaultPageSize instead of the magic number 20 to stay consistent.
| limit = 20 | |
| limit = model.DefaultPageSize |
All of the functionality below has been "Vibe Coded" using ChatGPT.
NOTE: Requires testing before merge!