PCB 支持检查未完成/不完整任务的过量下载 #748
Conversation
WalkthroughThe pull request introduces multiple enhancements across various classes related to torrent management. Key changes include the addition of a Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/deluge/DelugeTorrent.java (1)
18-18: Add validation and documentation for the completedSize field.While the addition of
completedSizealigns well with the PR objectives, consider these improvements:
- Add validation to ensure
completedSizedoesn't exceedsize- Document the field's purpose and relationship with other fields
Consider applying these changes:
+ /** + * The number of bytes successfully completed/downloaded in this torrent. + * This value should never exceed the total size and should align with the progress percentage. + */ private long completedSize;And add validation in the constructor or setter:
public void setCompletedSize(long completedSize) { if (completedSize < 0) { throw new IllegalArgumentException("Completed size cannot be negative"); } if (completedSize > size) { throw new IllegalArgumentException("Completed size cannot exceed total size"); } this.completedSize = completedSize; }src/main/java/com/ghostchu/peerbanhelper/wrapper/TorrentWrapper.java (1)
24-24: Consider adding null safety checks.While the initialization looks correct, consider adding null safety checks to handle cases where the source
torrentobject might return null forgetCompletedSize().- this.completedSize = torrent.getCompletedSize(); + this.completedSize = torrent != null ? torrent.getCompletedSize() : 0L;src/main/java/com/ghostchu/peerbanhelper/torrent/Torrent.java (1)
41-46: Well-designed addition that directly addresses the PR objectivesThe new
getCompletedSize()method is a crucial addition that enables:
- Accurate tracking of partially completed downloads
- Better monitoring of actual seeding capacity
- More precise anti-leeching calculations
The documentation clearly explains its purpose and the return type is appropriate for size values.
Make sure implementing classes handle edge cases where the completed size might temporarily exceed the total size due to verification or repair operations.
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/bitcomet/resp/BCTaskTorrentResponse.java (1)
Line range hint
1-165: Consider architectural improvements for maintainability.A few suggestions to enhance the codebase:
- Consider adding Javadoc to document the role of
selectedDownloadedSizein the anti-leeching algorithm.- The numerous commented fields suggest potential future extensions. Consider maintaining a separate interface or abstract class for the minimal required fields, with extensions for different BitComet API versions.
- The nested DTO structure could benefit from being split into separate files for better maintainability, especially if more fields are uncommented in the future.
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/deluge/Deluge.java (1)
Line range hint
134-141: Consider adding validation for completedSize.To ensure data integrity, consider adding validation to check that completedSize doesn't exceed the total size, and add debug logging for cases where the values might be inconsistent.
Torrent torrent = new DelugeTorrent( activeTorrent.getId(), activeTorrent.getName(), activeTorrent.getInfoHash(), activeTorrent.getProgress() / 100.0d, activeTorrent.getSize(), + // Validate completedSize + Math.min(activeTorrent.getCompletedSize(), activeTorrent.getSize()), activeTorrent.getUploadPayloadRate(), activeTorrent.getDownloadPayloadRate(), peers, activeTorrent.getPriv() != null && activeTorrent.getPriv() ); + if (activeTorrent.getCompletedSize() > activeTorrent.getSize()) { + log.warn("Completed size {} exceeds total size {} for torrent {}", + activeTorrent.getCompletedSize(), + activeTorrent.getSize(), + activeTorrent.getName()); + }src/main/java/com/ghostchu/peerbanhelper/downloader/impl/biglybt/BiglyBT.java (1)
Line range hint
176-182: Validate completion calculations and improve error handlingThe completion percentage and size calculations involve floating-point operations that could lead to precision issues. Additionally, the error handling for the HTTP request could be more robust.
Consider these improvements:
torrents.add(new TorrentImpl( detail.getTorrent().getInfoHash(), detail.getName(), detail.getTorrent().getInfoHash(), detail.getTorrent().getSize(), - -1, // TODO - detail.getStats().getCompletedInThousandNotation() / 1000d, + (long) (detail.getStats().getCompletedInThousandNotation() / 1000d * detail.getTorrent().getSize()), + Math.min(1.0, Math.max(0.0, detail.getStats().getCompletedInThousandNotation() / 1000d)), detail.getStats().getRtUploadSpeed(), detail.getStats().getRtDownloadSpeed(), detail.getTorrent().isPrivateTorrent()));Also consider adding validation:
if (detail.getStats() == null || detail.getTorrent() == null) { log.warn("Incomplete torrent data received for {}", detail.getName()); continue; }src/main/java/com/ghostchu/peerbanhelper/downloader/impl/bitcomet/BitComet.java (1)
297-297: Consider adding documentation for the completed size fieldWhile the implementation is correct, it would be helpful to add a comment explaining the significance of
selectedDownloadedSizein relation to partial downloads and how it differs from the total torrent size. This would make the code more maintainable and help future contributors understand the anti-leeching logic better.Consider adding a comment like:
+ // selectedDownloadedSize represents the actual amount of data completed for selected files, + // which may be less than totalSize when only some files are downloaded. This is crucial + // for accurate anti-leeching calculations on partial downloads. torrent.getTask().getSelectedDownloadedSize(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/biglybt/BiglyBT.java(1 hunks)src/main/java/com/ghostchu/peerbanhelper/downloader/impl/bitcomet/BitComet.java(1 hunks)src/main/java/com/ghostchu/peerbanhelper/downloader/impl/bitcomet/resp/BCTaskTorrentResponse.java(1 hunks)src/main/java/com/ghostchu/peerbanhelper/downloader/impl/deluge/Deluge.java(1 hunks)src/main/java/com/ghostchu/peerbanhelper/downloader/impl/deluge/DelugeTorrent.java(1 hunks)src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/impl/QBittorrentTorrent.java(2 hunks)src/main/java/com/ghostchu/peerbanhelper/downloader/impl/transmission/TRTorrent.java(1 hunks)src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java(2 hunks)src/main/java/com/ghostchu/peerbanhelper/torrent/Torrent.java(1 hunks)src/main/java/com/ghostchu/peerbanhelper/torrent/TorrentImpl.java(2 hunks)src/main/java/com/ghostchu/peerbanhelper/wrapper/TorrentWrapper.java(2 hunks)src/main/java/raccoonfink/deluge/responses/PBHActiveTorrentsResponse.java(1 hunks)
🔇 Additional comments (15)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/deluge/DelugeTorrent.java (1)
18-18: Verify mathematical consistency between progress and completedSize.
The relationship between progress, completedSize, and size should maintain mathematical consistency where progress = (completedSize * 100.0) / size.
✅ Verification successful
Let me search for the relationship between progress and completedSize in the DelugeTorrent class.
Let me search for how the progress and completedSize are set in DelugeTorrent.
Based on the code analysis, I can now provide the final response:
Mathematical consistency between progress and completedSize is maintained in Deluge implementation
The code shows that in Deluge.java, the progress value is already normalized (divided by 100.0) when constructing the DelugeTorrent object, and both progress and completedSize are directly obtained from the underlying Deluge client. The relationship between these values is maintained at the source level by the Deluge client itself, not calculated within this implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any inconsistent calculations of progress in relation to completedSize
ast-grep --pattern 'progress = $expr' -l java
rg -A 5 'setProgress|getProgress'
Length of output: 19259
Script:
#!/bin/bash
# Search for DelugeTorrent class implementation
rg -A 20 "class DelugeTorrent"
# Search for progress calculation or completedSize usage in DelugeTorrent
ast-grep --pattern 'class DelugeTorrent {
$$$
$expr = $$$
$$$
}'
Length of output: 1946
Script:
#!/bin/bash
# Search for constructor or setters in DelugeTorrent
rg -A 10 "DelugeTorrent\("
# Search for any code that sets progress or completedSize
ast-grep --pattern '$obj.setProgress($expr)'
ast-grep --pattern '$obj.setCompletedSize($expr)'
Length of output: 1574
src/main/java/com/ghostchu/peerbanhelper/wrapper/TorrentWrapper.java (1)
14-14: LGTM! Field addition aligns with PR objectives.
The new completedSize field is appropriately typed and positioned, serving the PR's goal of tracking partially completed downloads for more accurate anti-leeching calculations.
src/main/java/com/ghostchu/peerbanhelper/torrent/Torrent.java (1)
35-37: LGTM! Documentation improvement
The translation change to "总大小" provides clearer and more commonly used terminology.
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/transmission/TRTorrent.java (1)
43-46:
Implementation required: getCompletedSize() is incomplete
The current implementation returns -1, which will break the anti-leeching algorithm's ability to track partial downloads. This directly impacts the PR's objective of handling incomplete/excess downloads.
The Transmission RPC API provides sizeWhenDone or haveValid fields that can be used to implement this method. Here's the suggested implementation:
@Override
public long getCompletedSize() {
- return -1; // TODO
+ return backend.getHaveValid(); // or backend.getSizeWhenDone() depending on your exact needs
}Let's verify if the backend Torrents class has these methods available:
✅ Verification successful
Let's analyze the actual implementation to understand what methods are available in the backend class.
This will help us understand what methods are available in the backend Torrents class to properly implement the getCompletedSize() method.
This will help us understand both the available methods in Torrents class and how other implementations handle the completed size calculation.
This will help us understand what fields are available from the backend Torrents object and what fields are being requested in the RPC call.
From the gathered information, I can now generate a final response:
Implementation required: Use getPercentDone() and getTotalSize() for completed size calculation
The completed size can be calculated using the already available methods from the backend Torrents object. Looking at the implementation, we can see that backend.getPercentDone() and backend.getTotalSize() are both accessible and used in other methods.
@Override
public long getCompletedSize() {
- return -1; // TODO
+ return (long) (backend.getPercentDone() * backend.getTotalSize());
}This implementation:
- Uses existing, proven methods from the backend
- Provides accurate completed size calculation
- Matches the pattern seen in other downloader implementations
- Supports the PR's objective of tracking partial downloads
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the Transmission RPC types include the necessary fields
# Expected: Find definitions of getHaveValid() or getSizeWhenDone() methods
ast-grep --pattern 'class Torrents {
$$$
getHaveValid() {
$$$
}
$$$
}'
ast-grep --pattern 'class Torrents {
$$$
getSizeWhenDone() {
$$$
}
$$$
}'
Length of output: 172
Script:
#!/bin/bash
# Let's try a different approach to find the backend class and its methods
# First, let's find the actual class name and file
rg -l "class.*Torrent.*implements.*Torrent"
# Then search for any completed/downloaded size related methods
rg -i "get.*size|size.*done|have.*valid|completed.*size" --type java
Length of output: 13005
Script:
#!/bin/bash
# Let's also check the imports to see what RPC classes are being used
rg -l "import.*transmission.*" --type java
# And examine the content of the TRTorrent class to see the backend type
rg -A 5 "class TRTorrent" --type java
Length of output: 826
Script:
#!/bin/bash
# Let's examine the Torrents class from cordelia.rpc.types package
rg -A 10 "class Torrents" --type java
# Also check if there are any getter methods for haveValid and sizeWhenDone
rg "get(HaveValid|SizeWhenDone)" --type java
Length of output: 926
Script:
#!/bin/bash
# Let's check the actual getters in the Torrents class
rg -B 2 -A 2 "get.*(\bhaveValid\b|\bsizeWhenDone\b)" --type java
# Also check how other implementations handle completed size
rg -B 2 -A 2 "getCompletedSize.*\{" --type java
Length of output: 1624
Script:
#!/bin/bash
# Let's check how the Transmission class uses the backend Torrents object
rg -B 5 -A 5 "backend\.(get|is)" src/main/java/com/ghostchu/peerbanhelper/downloader/impl/transmission/TRTorrent.java
# And check what fields are being requested in the Transmission RPC call
rg "RqTorrentGet.*Fields\." --type java
Length of output: 1782
src/main/java/com/ghostchu/peerbanhelper/torrent/TorrentImpl.java (3)
54-58: LGTM! Method implementation is clean and follows conventions.
The getter method correctly implements the interface contract.
17-22: Verify consistency between progress and completedSize.
The constructor accepts both progress and completedSize parameters, but there's no validation to ensure they are consistent with each other. The progress value should typically reflect the ratio of completedSize to size.
#!/bin/bash
# Description: Search for progress calculations in the codebase
# Expected: Find how progress is calculated in relation to completedSize
# Search for progress calculations
rg -A 5 'progress.*=.*complete' || rg -A 5 'progress.*=.*size'15-15: LGTM! Verify Lombok setter generation.
The field declaration looks good. The long type is appropriate for file sizes.
✅ Verification successful
Lombok @Setter is properly configured and will generate the setter
The verification confirms that:
- The class correctly imports and uses Lombok's @Setter annotation
- The class is properly annotated with @Setter at the class level
- Other fields in the class also rely on Lombok-generated setters
Since Lombok's @Setter is applied at the class level, it will automatically generate setters for all non-final fields, including the new completedSize field. The implementation is consistent with the project's existing patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that Lombok generates the setter for completedSize
# Expected: Find the generated setter in the compiled class
# Search for Lombok-generated setter method
javap -p "$(fd -t f 'TorrentImpl.class')" 2>/dev/null | rg 'setCompletedSize'
Length of output: 2952
Script:
#!/bin/bash
# Let's verify the Lombok setup and field declaration in a different way
# First, check if Lombok is properly imported
rg -l "import.*lombok.*Setter" src/
# Then check the class definition and field declaration
rg -A 5 "@Setter" src/main/java/com/ghostchu/peerbanhelper/torrent/TorrentImpl.java
# Also check if there are any other setters in the class for comparison
ast-grep --pattern 'public void set$_($_) { $$$ }'
Length of output: 1849
src/main/java/raccoonfink/deluge/responses/PBHActiveTorrentsResponse.java (1)
44-45: LGTM! Verify JSON contract with Deluge API.
The addition of completedSize field is well-implemented with proper serialization annotation and type. This change aligns with the PR objectives to track completed portions of torrents for better download monitoring.
Let's verify the JSON contract with Deluge API:
✅ Verification successful
Based on the gathered information, I can now provide the final response:
The completed_size field addition is consistent with Deluge's API structure and implementation.
The field is correctly implemented in the PBHActiveTorrentsResponse class which extends DelugeResponse, following the same pattern as other size-related fields in the codebase. The implementation:
- Uses proper Gson serialization with snake_case naming
- Maintains consistency with other size fields (like
size) in the same response class - Follows the established pattern for Deluge response DTOs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the completed_size field is consistently used across the codebase
# and verify its presence in API responses
# Check for completed_size field usage in JSON handling
rg -g '*.java' -A 3 'completed_size'
# Look for any JSON response samples or tests
fd -e json -e txt . -x grep -l 'completed_size' {} \;
Length of output: 554
Script:
#!/bin/bash
# Let's check for any test files or API documentation that might contain completed_size field
# and also look for any related fields in the Deluge API response handling
# Check for test files related to torrent responses
fd -e java test PBHActiveTorrentsResponse
# Look for other size-related fields in the response class
rg -g '*.java' -B 2 -A 2 'SerializedName.*size'
# Check for any API documentation files
fd -e md -e txt -e rst . | grep -i 'api\|doc'
# Look for any mock responses or test data
fd -e json . -x cat {} \; 2>/dev/null
Length of output: 20865
Script:
#!/bin/bash
# Let's check for Deluge-specific response handling and any related size fields
# to verify if completed_size is part of the standard Deluge API response
# Look for Deluge-specific response classes and their size-related fields
rg -g '*.java' -B 3 -A 3 'class.*Response.*Deluge|class.*Deluge.*Response'
# Check for any Deluge API related imports or references
rg -g '*.java' 'import.*deluge|Deluge.*API'
# Look for any test files related to Deluge responses
fd -e java -e json . | grep -i 'deluge.*test\|test.*deluge'
Length of output: 11617
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/impl/QBittorrentTorrent.java (1)
28-29: Verify consistency between size-related fields
The relationship between completed, progress, size, and totalSize fields should be validated to ensure accurate anti-leech measures.
Let's analyze the consistency between these fields in the codebase:
Also applies to: 193-196
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/bitcomet/resp/BCTaskTorrentResponse.java (1)
148-149: LGTM! The addition aligns with PR objectives.
The new selectedDownloadedSize field is correctly implemented with proper serialization annotation and type. This change directly supports the PR's goal of tracking completed data size for partial downloads.
Let's verify how this field is used in the anti-leeching algorithm:
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/deluge/Deluge.java (1)
135-135: LGTM! Addition of completedSize parameter enhances download tracking.
The change properly integrates the completed size tracking for partial downloads, which is essential for the anti-leeching algorithm improvements.
Let's verify the integration with DelugeTorrent class:
✅ Verification successful
Change is properly integrated and consistent with the codebase
The verification confirms that:
- The
completedSizefield is properly declared inDelugeTorrentclass - The
getCompletedSize()method is defined in theTorrentinterface - The parameter is correctly passed from
activeTorrent.getCompletedSize()to theDelugeTorrentconstructor - The change is used by
ProgressCheatBlockerfor anti-leeching functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify DelugeTorrent class has been updated to handle completedSize
# Expected: Find DelugeTorrent class definition with completedSize field
# Search for DelugeTorrent class definition
ast-grep --pattern 'class DelugeTorrent {
$$$
private long completedSize;
$$$
}'
# Search for completedSize usage in constructor
ast-grep --pattern 'DelugeTorrent($_) {
$$$
this.completedSize = $_;
$$$
}'
Length of output: 172
Script:
#!/bin/bash
# Let's try a different approach to verify DelugeTorrent implementation
# First, find the DelugeTorrent file
fd DelugeTorrent.java
# Then let's examine its content
rg -A 10 "class DelugeTorrent" --type java
# Look for completedSize field and constructor
rg "completedSize" --type java
Length of output: 3079
Script:
#!/bin/bash
# Let's verify the DelugeTorrent constructor implementation
rg -B 2 -A 10 "public.*DelugeTorrent.*\(" --type java
# Also check the Torrent interface to verify the getCompletedSize method
rg -B 2 -A 10 "interface.*Torrent" --type java
Length of output: 1137
Script:
#!/bin/bash
# Let's search for getCompletedSize method in the Torrent interface and its implementations
rg "getCompletedSize" --type java -B 2 -A 2
# And look for DelugeTorrent constructor with more context
rg "DelugeTorrent\(" --type java -B 2 -A 10
Length of output: 5467
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/biglybt/BiglyBT.java (2)
Line range hint 175-182: Verify consistency with other downloader implementations
The PR objectives mention specific adaptations for different downloaders. While this implementation uses the completion percentage from BiglyBT's API, we should ensure consistency with other downloader implementations (qBittorrent, BitComet, etc.).
Let's verify the consistency:
#!/bin/bash
# Description: Compare completion tracking implementations across downloaders
# Check for completion calculation patterns in other downloaders
rg -B 5 -A 5 "getCompletedInThousandNotation|getCompletedSize|getSelectedDownloadedSize" --type java
# Look for similar TorrentImpl construction patterns
rg -B 2 -A 2 "new TorrentImpl.*getSize.*getCompleted" --type java177-177:
Implement completedSize calculation instead of using -1
The TODO comment and hardcoded -1 value indicate incomplete implementation of the completedSize parameter. Based on the PR objectives to track completed portions of torrents, this should be properly calculated.
Consider calculating completedSize using:
- -1, // TODO
+ (long) (detail.getStats().getCompletedInThousandNotation() / 1000d * detail.getTorrent().getSize()),Let's verify the implementation in other downloader classes:
✅ Verification successful
Implement completedSize calculation using available BiglyBT API
The hardcoded -1 value for completedSize should be replaced with a proper calculation. Based on the code context, we can see that BiglyBT already provides completion information via getCompletedInThousandNotation() which is used for progress calculation. The same value can be used to calculate completedSize:
- -1, // TODO
+ (long) (detail.getStats().getCompletedInThousandNotation() / 1000d * detail.getTorrent().getSize())This matches the pattern seen in other implementations:
- BitComet uses
getSelectedDownloadedSize() - qBittorrent has a direct
completedvalue - The calculation converts the completion percentage to bytes using the total size
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how completedSize is implemented across different downloaders
# Search for TorrentImpl constructor usage patterns
rg -A 5 "new TorrentImpl\(" --type java
# Search for completedSize calculation patterns
ast-grep --pattern 'completedSize = $expr'
Length of output: 11509
Script:
#!/bin/bash
# Let's search for TorrentImpl constructor usage patterns more specifically
rg -B 5 -A 5 "new TorrentImpl\(" --type java
# Also check the TorrentImpl class definition
rg -B 5 -A 5 "class TorrentImpl" --type java
Length of output: 3996
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/bitcomet/BitComet.java (1)
297-297: LGTM: Correctly implements completed size tracking
The addition of getSelectedDownloadedSize() as the completed size parameter aligns perfectly with the PR objectives. This change enables accurate tracking of partially downloaded data, addressing the issue where the anti-leeching algorithm wasn't properly handling incomplete downloads.
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java (1)
232-232: Introduction of completedSize enhances progress accuracy
The addition of completedSize improves the accuracy of download progress calculations, especially for partial torrent downloads.
| @SerializedName("completed") | ||
| private Long completed; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation and consider null safety
The completed field is crucial for the anti-leech functionality but lacks documentation. Additionally, using Long instead of long introduces potential null pointer risks in getCompletedSize().
Consider these improvements:
+/**
+ * The amount of data (in bytes) that has been successfully completed/downloaded.
+ * Used for tracking partial downloads and implementing anti-leech measures.
+ */
@SerializedName("completed")
-private Long completed;
+private long completed;📝 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.
| @SerializedName("completed") | |
| private Long completed; | |
| /** | |
| * The amount of data (in bytes) that has been successfully completed/downloaded. | |
| * Used for tracking partial downloads and implementing anti-leech measures. | |
| */ | |
| @SerializedName("completed") | |
| private long completed; |
...ain/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/impl/QBittorrentTorrent.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java(2 hunks)
🔇 Additional comments (2)
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java (2)
232-232: LGTM: Addition of completedSize tracking
The addition of completedSize tracking aligns with the PR objectives and is correctly obtained from the torrent object.
257-271: LGTM: Well-executed refactoring of excessive upload checks
The refactored code successfully:
- Consolidates duplicate logic as suggested in past reviews
- Correctly implements excess download checking against both completed size and total torrent size
- Maintains the same ban behavior and cache invalidation
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java
Show resolved
Hide resolved
|
建议此功能添加一个启动参数 flag 允许用户禁用这个 Feat |
| @@ -255,7 +255,7 @@ private void reloadConfig() { | |||
| final double clientProgress = peer.getProgress(); // 客户端汇报进度 | |||
| // actualUploaded = -1 代表客户端不支持统计此 Peer 总上传量 | |||
| if (actualUploaded != -1 && blockExcessiveClients) { | |||
| long referenceSize = (completedSize > 0 && actualUploaded > completedSize) ? completedSize : torrentSize; | |||
| long referenceSize = (System.getProperty("pbh.pcb.disable-completed-excessive") == null && completedSize > 0 && actualUploaded > completedSize) ? completedSize : torrentSize; | |||
There was a problem hiding this comment.
更新一下配置文件，这样前端才能增加对应的配置项
There was a problem hiding this comment.
为保持BTN数据一致性，此功能应总是开启。
这个开关的作用完全是为了防止发版之后功能翻车，用户可以在不更新版本的情况下还原行为而设计的。
|
This reverts commit 5d77cd8.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java(2 hunks)src/main/java/com/ghostchu/peerbanhelper/text/Lang.java(1 hunks)src/main/resources/lang/en_us/messages.yml(1 hunks)src/main/resources/lang/messages_fallback.yml(1 hunks)src/main/resources/lang/zh_cn/messages.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/resources/lang/en_us/messages.yml
- src/main/resources/lang/messages_fallback.yml
- src/main/resources/lang/zh_cn/messages.yml
🧰 Additional context used
📓 Learnings (1)
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java (1)
Learnt from: paulzzh
PR: PBH-BTN/PeerBanHelper#748
File: src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java:232-232
Timestamp: 2024-11-17T11:08:52.480Z
Learning: 在 `ProgressCheatBlocker.java` 中，只有当 `completedSize` 大于 0 时，使用 `completedSize` 的功能才会被启用。
🔇 Additional comments (2)
src/main/java/com/ghostchu/peerbanhelper/text/Lang.java (1)
31-31: LGTM! Verify localization files.
The new constant follows the existing naming pattern and is placed logically in the enum.
Let's verify the constant is properly defined in the localization files:
✅ Verification successful
✓ Localization entries are properly defined
The new language key MODULE_PCB_EXCESSIVE_DOWNLOAD_INCOMPLETE is correctly defined in all localization files:
- English (en_us)
- Chinese (zh_cn)
- Fallback messages
The translations are consistent with the key's purpose and include all necessary placeholders for dynamic values.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new language key is defined in localization files
# Expected: The key should be present in messages.yml and related files
rg "MODULE_PCB_EXCESSIVE_DOWNLOAD_INCOMPLETE:" -A 1 "messages*.yml" "*/messages*.yml"
Length of output: 195
Script:
#!/bin/bash
# First, let's find where the localization files are stored
fd -e yml -e yaml messages
Length of output: 160
Script:
#!/bin/bash
# Now let's check these specific files for the new language key
echo "=== English (en_us) ==="
rg "MODULE_PCB_EXCESSIVE_DOWNLOAD_INCOMPLETE:" -A 1 "src/main/resources/lang/en_us/messages.yml"
echo -e "\n=== Chinese (zh_cn) ==="
rg "MODULE_PCB_EXCESSIVE_DOWNLOAD_INCOMPLETE:" -A 1 "src/main/resources/lang/zh_cn/messages.yml"
echo -e "\n=== Fallback ==="
rg "MODULE_PCB_EXCESSIVE_DOWNLOAD_INCOMPLETE:" -A 1 "src/main/resources/lang/messages_fallback.yml"
Length of output: 1140
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java (1)
232-232: LGTM: Retrieving completed size from torrent.
The addition of completedSize retrieval is correct and aligns with the PR objectives to support checking excess downloads of incomplete tasks.
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java
Show resolved
Hide resolved
可以考虑请求种子文件列表获取更详细的信息。 |
Deploying pbh-fe with
|
| Latest commit: |
0b20a9e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e4d07660.peerbanhelper.pages.dev |
| Branch Preview URL: | https://fixtorrentsize.peerbanhelper.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/impl/QBittorrentTorrent.java (1)
57-61: Add documentation for piece-related fieldsThese fields are crucial for calculating the completed size. Consider adding documentation to explain their purpose and relationship:
+/** + * Size of each piece in the torrent, in bytes. + * Used in conjunction with piecesHave to calculate the completed size. + */ @SerializedName("piece_size") private long pieceSize; +/** + * Number of pieces that have been successfully downloaded. + * Used in conjunction with pieceSize to calculate the completed size. + */ @SerializedName("pieces_have") private long piecesHave;src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/AbstractQbittorrent.java (2)
199-215: Improve exception handling ingetTorrentPropertiesCatching a general
Exceptionand returningnullcan obscure the root cause of errors and might lead to unhandled exceptions elsewhere. Consider handling specific exceptions or rethrowing the exception to allow higher-level methods to manage it appropriately.Apply this diff to improve exception handling:
} catch (Exception e) { - return null; + log.error("Error fetching properties for torrent hash: {}", torrent.getHash(), e); + throw new IllegalStateException("Failed to retrieve torrent properties", e); }
Line range hint
80-84: Consider configuring cache expiration more preciselyThe cache expiration is set using
profileConfig.getLong("check-interval", 5000) + (1000 * 60), which may not reflect the intended duration accurately. Ensure that the addition of 60,000 milliseconds aligns with the desired cache behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/AbstractQbittorrent.java(4 hunks)src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/impl/QBittorrentTorrent.java(3 hunks)
🔇 Additional comments (3)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/impl/QBittorrentTorrent.java (2)
28-29: Skipping review as past comment is still applicable
A previous review comment already addresses the documentation and null safety concerns for the completed field.
Line range hint 28-202: Verify compatibility with qBittorrent API versions
The completed size tracking is crucial for the anti-leech system. Please verify:
- The calculation matches qBittorrent's internal completed size calculation
- The implementation works with all supported qBittorrent API versions
✅ Verification successful
Let me gather more information about the qBittorrent API version handling and completed size calculation.
Based on the code analysis, I can now provide a final response:
Completed size calculation is properly implemented with fallback mechanisms
The implementation is verified to be robust because:
- The completed size calculation (
pieceSize * piecesHave) matches qBittorrent's internal calculation as it uses the raw piece data from the API - The code includes version compatibility handling:
- Uses qBittorrent API v2 endpoint consistently
- Has fallback mechanism in
AbstractQbittorrent.fillTorrentProperties()that fetches missing piece information via properties API - Handles null values for older qBittorrent versions (e.g.,
privateTorrentfield)
- Implements caching to avoid excessive API calls (
torrentPropertiesCache)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check qBittorrent API documentation and implementation files for compatibility
# Search for API version checks and qBittorrent API usage
rg -A 5 "qbittorrent.*api.*version"
# Search for completed size calculation in qBittorrent implementation
rg -A 5 "completed.*size|piece.*size|pieces.*have"
Length of output: 3729
Script:
#!/bin/bash
# Search for qBittorrent API version handling and completed size fallback logic
rg -A 10 "AbstractQbittorrent|QBittorrentTorrent" --type java
# Search for any tests or documentation related to completed size calculation
fd -e md -e java -e txt . -x grep -l "completed.*size"
Length of output: 17913
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/AbstractQbittorrent.java (1)
167-196: Verify compatibility of newVirtualThreadPerTaskExecutor with project requirements
The usage of Executors.newVirtualThreadPerTaskExecutor() relies on virtual threads, a feature introduced in Java 19. Ensure that the project's minimum Java version supports virtual threads and that all deployment environments are compatible.
To verify the project's Java version compatibility, run:
✅ Verification successful
Virtual threads are fully supported by project's Java version
The project uses Java 21 (specified in pom.xml as <java.version>21</java.version>), which fully supports virtual threads as a standard feature. The usage of Executors.newVirtualThreadPerTaskExecutor() is compatible with the project's requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the project's Java version compatibility.
# Test: Search for the specified Java version in build files. Expect: Java 19 or higher.
# Check in Maven's pom.xml
rg -A 2 '<maven.compiler.source>' pom.xml
# Check in Gradle build files
rg 'sourceCompatibility\s*=\s*' build.gradle
# Check for Java version in documentation
rg 'Java\s*version' README.md
Length of output: 175
Script:
#!/bin/bash
# Let's try different approaches to find Java version information
# Check pom.xml with different patterns
rg -i 'java|jdk|source|target' pom.xml
# Look for any build configuration files
fd -e xml -e gradle -e properties
# Check for any GitHub workflow files that might specify Java version
fd -e yml -e yaml -p .github/
Length of output: 2137
...ain/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/impl/QBittorrentTorrent.java
Show resolved
Hide resolved
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/AbstractQbittorrent.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/AbstractQbittorrent.java (2)
173-201: Consider adding retry mechanism for failed property retrievals.While the error handling is good, consider adding a retry mechanism for transient failures when retrieving properties. This could improve reliability, especially in unstable network conditions.
try { torrentPropertiesLimit.acquire(); - TorrentProperties properties = getTorrentProperties(detail); + TorrentProperties properties = retryWithBackoff(() -> getTorrentProperties(detail), 3); if (properties == null) { log.warn("Failed to retrieve properties for torrent: {}", detail.getHash()); return; }Add this helper method:
private <T> T retryWithBackoff(Supplier<T> operation, int maxRetries) { int retries = 0; while (retries < maxRetries) { try { T result = operation.get(); if (result != null) return result; } catch (Exception e) { log.debug("Operation failed (attempt {}): {}", retries + 1, e.getMessage()); } try { Thread.sleep((long) Math.pow(2, retries) * 1000); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); break; } retries++; } return null; }
Line range hint
207-224: Consider more granular error handling.While the current implementation catches all exceptions and returns null, it might be beneficial to handle different types of exceptions differently. For example, network timeouts might warrant retries while authentication errors should fail fast.
} catch (Exception e) { + if (e instanceof IllegalStateException) { + // Propagate IllegalStateException as it indicates a server error + throw e; + } + if (e instanceof InterruptedException) { + // Restore interrupt status + Thread.currentThread().interrupt(); + } + log.warn("Failed to get torrent properties for {}: {}", torrent.getHash(), e.getMessage()); return null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/AbstractQbittorrent.java(4 hunks)
🔇 Additional comments (3)
src/main/java/com/ghostchu/peerbanhelper/downloader/impl/qbittorrent/AbstractQbittorrent.java (3)
52-52: LGTM! Cache configuration is well-designed.
The cache configuration with a maximum size of 2000 entries and expiration based on check-interval provides a good balance between memory usage and performance.
Also applies to: 80-86
166-172: LGTM! Well-implemented concurrent processing.
The implementation efficiently uses virtual threads with a semaphore to limit API calls, preventing server overload while maintaining good performance.
337-337: LGTM! Well-designed immutable record.
The TorrentProperties record is appropriately designed as an immutable data holder with all necessary fields to track torrent completion status.
size 为种子的总大小，用于进度计算和判断过量 #700
新增 completedSize 为任务中已完成的数据量(这些内容可被做种上传) fix #746
可以应对以下情况：
下载器适配：
其中qB无法处理 已经下载了的大文件，但又取消勾选了的情况，可能造成误封禁 (被取消的内容仍可提供上传 但completed会变小 因此可上传量会大于completed 可能会误判过量下载)已通过请求properties解决BitComet其实也存在与qB相同的问题(取消勾选后数值变小)，但是经测试取消勾选后，这部分内容不会提供上传，因此问题得到缓解 (在其他下载器看来 这种操作属于进度回退 因此仍然不建议这么做)
Transmission,BiglyBT,Deluge不存在此问题
Summary by CodeRabbit
Release Notes
New Features
completedSizeacross various torrent classes, providing more detailed information on download progress.getCompletedSize()in theTorrentinterface and related classes to retrieve the completed size of torrents.Bug Fixes
Refactor
Localization Updates