Skip to content

Conversation

@Cucumberrbob
Copy link
Collaborator

@Cucumberrbob Cucumberrbob commented Jun 15, 2025

closes #838

Adds a new downloader: the .strm downloader.

This PR is currently marked as draft as in its current state it won't work well with TorBox and I haven't tested with anything other than AD so there may be other issues.

I'd like to also fi​x #336 and use the AD "Save Link" feature for .strm downloads
TorBox really should use the redirect=true links since the unrestricted ones only last a few hours.
I don't know about quirks of DL or PM yet.

@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

838 - PR Code Verified

Compliant requirements:

• Implement ability to download physical .strm files instead of full video files
• Create a separate internal download client for .strm files
• Handle .strm files exactly like video files (same download process)

Requires further human verification:

• Remove torrent from client and provider post download
• Support Sonarr handling the import of .strm files
• Enable removal of downloads from RD library after download
• Eliminate overhead of managing mounting rclone share

336 - Not compliant

Non-compliant requirements:

• Add option in RDT settings to save links on Alldebrid
• Support Alldebrid "Save Link" feature

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Return Value

The Download method returns a random GUID string which doesn't represent any meaningful download identifier or status, potentially breaking expected behavior for callers expecting a proper download ID or path.

    return Guid.NewGuid().ToString();
}
Missing Validation

No validation is performed on the downloadLink parameter before writing to file, which could result in invalid .strm files if the link is malformed or empty.

await fileSystem.File.WriteAllTextAsync(filePath + ".strm", downloadLink);

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Validate URL before file creation

The code doesn't validate if the downloadLink is a valid URL before writing it
to the file. Adding URL validation would prevent creating invalid .strm files
that media players cannot process.

server/RdtClient.Service/Services/Downloaders/StrmDownloader.cs [14]

+if (!Uri.TryCreate(downloadLink, UriKind.Absolute, out _))
+{
+    throw new ArgumentException($"Invalid download link: {downloadLink}");
+}
+
 await fileSystem.File.WriteAllTextAsync(filePath + ".strm", downloadLink);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This is a good suggestion for improving the robustness of the StrmDownloader. Validating that the downloadLink is a valid URL before writing it to a .strm file prevents creating corrupt files and allows for earlier, more specific error reporting. The implementation using Uri.TryCreate is correct.

Medium
Return meaningful file path value

The method returns a random GUID regardless of success or failure, which doesn't
provide meaningful information about the download operation. Consider returning
the actual file path on success or null/empty string on failure to make the
return value more informative.

server/RdtClient.Service/Services/Downloaders/StrmDownloader.cs [10-28]

 public async Task<String> Download()
 {
     try
     {
-        await fileSystem.File.WriteAllTextAsync(filePath + ".strm", downloadLink);
+        var strmFilePath = filePath + ".strm";
+        await fileSystem.File.WriteAllTextAsync(strmFilePath, downloadLink);
 
         DownloadComplete?.Invoke(this, new());
+        return strmFilePath;
     }
     catch (Exception ex)
     {
         DownloadComplete?.Invoke(this,
                                  new()
                                  {
                                      Error = ex.Message
                                  });
+        return String.Empty;
     }
-
-    return Guid.NewGuid().ToString();
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that returning a random GUID from the Download method is not very informative. Proposing to return the created file path on success and an empty string on failure makes the method's return value more useful for the caller, which is a good design improvement.

Low
  • More

@Cucumberrbob Cucumberrbob marked this pull request as draft June 15, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat Req : Download .strm files instead of the full video

1 participant