feat(impl): add FLD support#251
Conversation
📝 WalkthroughWalkthroughAdds Flood (FLD) tracker support for uploads, dry runs, duplicate checking, configuration, registry discovery, banned groups, UI fields, and tracker-specific description formatting. ChangesFlood tracker support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TrackerDefinition
participant UploadPreparation
participant FloodAPI
TrackerDefinition->>UploadPreparation: Build FLD upload request
UploadPreparation->>FloodAPI: POST multipart torrent upload
FloodAPI-->>UploadPreparation: Return upload result
UploadPreparation-->>TrackerDefinition: Return UploadSummary
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/dupechecking/fld_test.go (2)
195-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
strings.Containsfrom the standard library over custom helpers.
stringsContainsandstringsContainsSimplereimplementstrings.Contains. Import thestringspackage and replace these helpers to reduce maintenance burden and improve clarity.♻️ Proposed refactor
import ( "context" "io" "net/http" "net/http/httptest" + "strings" "testing" "github.com/autobrr/upbrr/internal/config" "github.com/autobrr/upbrr/pkg/api" )Then replace the usage at line 65:
- if len(notes) != 1 || !stringsContains(notes[0], tc.expectedSkip) { + if len(notes) != 1 || !strings.Contains(notes[0], tc.expectedSkip) {And remove lines 195-206:
-func stringsContains(s, sub string) bool { - return len(s) >= len(sub) && (s == sub || stringsContainsSimple(s, sub)) -} - -func stringsContainsSimple(s, sub string) bool { - for i := 0; i <= len(s)-len(sub); i++ { - if s[i:i+len(sub)] == sub { - return true - } - } - return false -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/dupechecking/fld_test.go` around lines 195 - 206, Replace usages of the custom stringsContains helper with the standard-library strings.Contains, adding the strings import as needed. Remove both stringsContains and stringsContainsSimple from the test file while preserving the existing containment behavior.
14-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for FLD response failures
Cover the missing branches inSearch: non-2xx responses, non-object JSON payloads, anditemsvalues that aren’t slices. That would lock down the defensive handling around these cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/dupechecking/fld_test.go` around lines 14 - 193, Extend the FLD Search tests around TestFLDHandlerSearchMovie and TestFLDHandlerSearchTV to cover non-2xx HTTP responses, valid JSON payloads that are not objects, and object payloads whose items field is not a slice. Assert each case preserves the defensive Search behavior by returning no entries and the expected error or handling result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/trackers/impl/fld/definition_test.go`:
- Around line 26-28: Update both torrent fixtures used by
ResolveUploadTorrentPath to contain valid bencode: correct the announce string
length prefix from 31 to the actual 25-byte URL in each os.WriteFile payload.
Keep the fixture contents and test intent otherwise unchanged so the normal
cleaned-torrent handling path is exercised.
In `@internal/trackers/impl/fld/upload.go`:
- Around line 341-346: Update resolveTMDbID to return an empty string
immediately when meta.ExternalIDs.TMDBID is less than or equal to zero;
otherwise preserve the existing TV/movie prefix formatting. Add tests covering
missing or non-positive TMDb IDs and confirming valid IDs still resolve
correctly.
---
Nitpick comments:
In `@internal/dupechecking/fld_test.go`:
- Around line 195-206: Replace usages of the custom stringsContains helper with
the standard-library strings.Contains, adding the strings import as needed.
Remove both stringsContains and stringsContainsSimple from the test file while
preserving the existing containment behavior.
- Around line 14-193: Extend the FLD Search tests around
TestFLDHandlerSearchMovie and TestFLDHandlerSearchTV to cover non-2xx HTTP
responses, valid JSON payloads that are not objects, and object payloads whose
items field is not a slice. Assert each case preserves the defensive Search
behavior by returning no entries and the expected error or handling result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3a1dd195-5ee3-4d78-88d2-90f5160ffd36
📒 Files selected for processing (15)
gui/frontend/src/hooks/useSettingsState.tsxinternal/config/defaults/example.yamlinternal/dupechecking/factory.gointernal/dupechecking/fld.gointernal/dupechecking/fld_test.gointernal/services/bbcode/tracker_profiles.gointernal/services/bbcode/tracker_profiles_test.gointernal/trackers/banned.gointernal/trackers/catalog.gointernal/trackers/impl/fld/definition.gointernal/trackers/impl/fld/definition_test.gointernal/trackers/impl/fld/upload.gointernal/trackers/impl/registry.gointernal/trackers/impl/registry_test.gointernal/trackers/upload_artifact.go
| if err := os.WriteFile(torrentPath, []byte("d8:announce31:http://localhost/announcee"), 0o600); err != nil { | ||
| t.Fatalf("write torrent: %v", err) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use valid bencode torrent fixtures.
The announce URL is 25 bytes, not 31. These malformed torrents make ResolveUploadTorrentPath take its load-error fallback instead of exercising normal cleaned-torrent handling.
Proposed fix
- []byte("d8:announce31:http://localhost/announcee"),
+ []byte("d8:announce25:http://localhost/announcee"),Apply this to both fixtures.
Also applies to: 97-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/trackers/impl/fld/definition_test.go` around lines 26 - 28, Update
both torrent fixtures used by ResolveUploadTorrentPath to contain valid bencode:
correct the announce string length prefix from 31 to the actual 25-byte URL in
each os.WriteFile payload. Keep the fixture contents and test intent otherwise
unchanged so the normal cleaned-torrent handling path is exercised.
| func resolveTMDbID(meta api.PreparedMetadata) string { | ||
| if resolveCategory(meta) == "TV" { | ||
| return fmt.Sprintf("tv/%d", meta.ExternalIDs.TMDBID) | ||
| } | ||
| return fmt.Sprintf("movie/%d", meta.ExternalIDs.TMDBID) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return no TMDb field when the ID is absent.
A zero ID becomes movie/0 or tv/0, so the missing-ID gate at Line 204 never blocks and uploads with a valid IMDb ID can still submit an invalid TMDb ID. Return "" when TMDBID <= 0, and cover the no-TMDb case.
Proposed fix
func resolveTMDbID(meta api.PreparedMetadata) string {
+ if meta.ExternalIDs.TMDBID <= 0 {
+ return ""
+ }
if resolveCategory(meta) == "TV" {
return fmt.Sprintf("tv/%d", meta.ExternalIDs.TMDBID)
}As per coding guidelines, “add tests for changed behavior.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func resolveTMDbID(meta api.PreparedMetadata) string { | |
| if resolveCategory(meta) == "TV" { | |
| return fmt.Sprintf("tv/%d", meta.ExternalIDs.TMDBID) | |
| } | |
| return fmt.Sprintf("movie/%d", meta.ExternalIDs.TMDBID) | |
| } | |
| func resolveTMDbID(meta api.PreparedMetadata) string { | |
| if meta.ExternalIDs.TMDBID <= 0 { | |
| return "" | |
| } | |
| if resolveCategory(meta) == "TV" { | |
| return fmt.Sprintf("tv/%d", meta.ExternalIDs.TMDBID) | |
| } | |
| return fmt.Sprintf("movie/%d", meta.ExternalIDs.TMDBID) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/trackers/impl/fld/upload.go` around lines 341 - 346, Update
resolveTMDbID to return an empty string immediately when meta.ExternalIDs.TMDBID
is less than or equal to zero; otherwise preserve the existing TV/movie prefix
formatting. Add tests covering missing or non-positive TMDb IDs and confirming
valid IDs still resolve correctly.
Source: Coding guidelines
FLD is a relatively new tracker with a custom source (mizo)
i've used the existing python implementation and the go one for bhd as a referenece
Summary by CodeRabbit