Conversation
[pull] origin from krau:main
…or Minio and Webdav
[pull] origin from krau:main
Reviewer's GuideThis PR implements comprehensive support for Telegram media groups (albums): messages are collected by group ID with a debounce timer, then processed together as batch or single file tasks; the rule engine is extended with a new IS-ALBUM rule and matched directory paths; batch file task creation now handles albums as folders; along with related updates to handlers, storage drivers, docs, CI workflows, Dockerfile caching, config, and constants. Sequence diagram for handling Telegram media group (album) messagessequenceDiagram
participant User as actor User
participant Bot as Bot
participant MediaGroupHandler as MediaGroupHandler
participant Storage as Storage
participant RuleEngine as RuleEngine
User->>Bot: Send media group messages
Bot->>MediaGroupHandler: handleGroupMediaMessage(ctx, update, message, groupID)
MediaGroupHandler->>MediaGroupHandler: Collect files by groupID
MediaGroupHandler->>MediaGroupHandler: Start debounce timer
MediaGroupHandler-->>User: (No immediate response)
Note over MediaGroupHandler: After timeout
MediaGroupHandler->>MediaGroupHandler: processMediaGroup(ctx, update, groupID)
MediaGroupHandler->>RuleEngine: Apply IS-ALBUM rule
RuleEngine-->>MediaGroupHandler: Return storage and dir path
MediaGroupHandler->>Storage: Create batch file task (album as folder)
Storage-->>MediaGroupHandler: Store files
MediaGroupHandler->>Bot: Edit message to show result
Bot-->>User: Notify user of save result
ER diagram for IS-ALBUM rule and album directory handlingerDiagram
RULE {
id int
type string
storage_name string
dir_path string
data string
}
ALBUM {
group_id int
dir_name string
}
FILE {
id int
group_id int
name string
storage_path string
}
RULE ||--o{ FILE : applies
ALBUM ||--|{ FILE : contains
FILE }o--|| ALBUM : belongs_to
Class diagram for new and updated rule types (IS-ALBUM and MatchedDirPath)classDiagram
class RuleMediaType {
- storName string
- storPath string
- matchAlbum bool
+ Type() RuleType
+ Match(input bool) (bool, error)
+ StorageName() string
+ StoragePath() string
}
class MatchedDirPath {
+ String() string
+ NeedNewForAlbum() bool
}
RuleMediaType --|> RuleClass
RuleClass <|.. RuleMediaType
MatchedDirPath <.. ApplyRule
ApplyRule --> MatchedDirPath : returns
ApplyRule --> RuleMediaType : uses
Class diagram for MediaGroupHandler and media group processingclassDiagram
class MediaGroupHandler {
- groups map[int64][]TGFileMessage
- timers map[int64]*time.Timer
- mu sync.Mutex
- timeout time.Duration
+ handleGroupMediaMessage(ctx, update, message, groupID) error
+ processMediaGroup(ctx, update, groupID)
}
class TGFileMessage
MediaGroupHandler "1" o-- "*" TGFileMessage : groups
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce support for album (media group) handling in Telegram bot media processing, including new rule types and directory structuring for grouped media. Workflow improvements, configuration enhancements, and documentation updates accompany these features. Additional safeguards for file storage uniqueness and minor refactoring are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bot
participant MediaGroupHandler
participant Storage
User->>Bot: Send media group messages (album)
Bot->>MediaGroupHandler: Detects group, stores files by group ID
MediaGroupHandler-->>MediaGroupHandler: Reset 1s timer per group
MediaGroupHandler->>Bot: On timer expiry, batch process group
Bot->>Storage: Save all files in group (possibly in new album folder)
Storage-->>Bot: Confirm save
Bot->>User: Reply with save status
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (17)
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Silentely, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily introduces comprehensive support for handling Telegram media groups (albums), allowing users to define sophisticated rules for how these grouped files are stored, including automatic directory creation. It also includes several quality-of-life improvements such as configurable API retries, more efficient Docker builds, and more reliable unique filename generation for stored files.
Highlights
- Media Group (Album) Handling: Implemented robust handling for Telegram media groups (albums), allowing the bot to process and save multiple files sent as a single album. This includes a timeout mechanism to collect all parts of an album before processing.
- Advanced Rule-Based Storage for Albums: Introduced a new rule type,
IS-ALBUM, which enables users to define specific storage rules for media groups. Additionally, a specialNEW-FOR-ALBUMpath option allows the bot to automatically create a new subdirectory for each album, named after the first file in the group. - Configurable RPC Retries: The number of RPC retries for Telegram API calls is now configurable via the
rpc_retrysetting in thetelegramsection of the configuration, providing more control over network resilience. - Improved Docker Build Efficiency: The Dockerfile has been optimized to leverage Go module caching, significantly speeding up subsequent Docker builds by separating the
go mod downloadstep. - Enhanced Unique Filename Generation: Storage backends (Minio and WebDAV) now include a more robust fallback mechanism to generate unique filenames using
xidif the standard sequential numbering approach fails after many attempts, preventing potential naming conflicts.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Silentely - I've reviewed your changes - here's some feedback:
- In CreateAndAddBatchTGFileTaskWithEdit you build album tasks using the outer-scoped
dirPath, which may have been overwritten in the loop; consider storing each file's computeddirPathalongside it so you don’t rely on a single loop variable when creating album directories. - The timer callback in
handleGroupMediaMessagecaptures the originalctxandupdateby reference, which can lead to stale or invalid state when it fires; extract only the necessary values into the closure or clone the data to avoid race conditions. - While MediaGroupHandler protects its maps with a mutex, the processing function runs asynchronously without coordinating with the main dispatcher; ensure any shared state or context operations in
processMediaGroupare properly synchronized or error-handled to prevent data races.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In CreateAndAddBatchTGFileTaskWithEdit you build album tasks using the outer-scoped `dirPath`, which may have been overwritten in the loop; consider storing each file's computed `dirPath` alongside it so you don’t rely on a single loop variable when creating album directories.
- The timer callback in `handleGroupMediaMessage` captures the original `ctx` and `update` by reference, which can lead to stale or invalid state when it fires; extract only the necessary values into the closure or clone the data to avoid race conditions.
- While MediaGroupHandler protects its maps with a mutex, the processing function runs asynchronously without coordinating with the main dispatcher; ensure any shared state or context operations in `processMediaGroup` are properly synchronized or error-handled to prevent data races.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces significant new features, including support for Telegram media groups and userbot integration, along with several other enhancements. I've found two high-severity correctness bugs in the new album-handling logic that could lead to files not being saved or being saved in the wrong location. I've also provided several medium-severity recommendations to improve maintainability. Addressing these points will greatly improve the quality and robustness of the new features.
| if len(afiles) <= 1 { | ||
| continue | ||
| } |
There was a problem hiding this comment.
The current logic skips processing for albums that contain only one file (len(afiles) <= 1). This means if a user sends a single file that matches an IS-ALBUM rule with NEW-FOR-ALBUM, the file will be dropped and not saved. This is a bug, as the user would expect the file to be saved, even if it's a single-item album. To fix this, you should remove this condition to ensure all matching files are processed.
| albumDir := strings.TrimSuffix(path.Base(afiles[0].file.Name()), path.Ext(afiles[0].file.Name())) | ||
| albumStor := afiles[0].storage | ||
| for _, af := range afiles { | ||
| afstorPath := af.storage.JoinStoragePath(path.Join(dirPath, albumDir, af.file.Name())) |
There was a problem hiding this comment.
There is a logic error in how the storage path is constructed for album files. The dirPath variable is not being used correctly when joining the storage path. The path should be constructed from the generated albumDir and the filename.
| afstorPath := af.storage.JoinStoragePath(path.Join(dirPath, albumDir, af.file.Name())) | |
| afstorPath := af.storage.JoinStoragePath(path.Join(albumDir, af.file.Name())) |
| if mediaGroupHandler.groups[groupID] == nil { | ||
| mediaGroupHandler.groups[groupID] = make([]tfile.TGFileMessage, 0) | ||
| } |
There was a problem hiding this comment.
The check for a nil slice before appending is redundant. In Go, append can be safely called on a nil slice, and it will automatically allocate a new underlying array. Removing this check simplifies the code.
mediaGroupHandler.groups[groupID] = append(mediaGroupHandler.groups[groupID], file)| candidate := storagePath | ||
| for i := 1; m.Exists(ctx, candidate); i++ { | ||
| candidate = fmt.Sprintf("%s_%d%s", base, i, ext) | ||
| if i > 1000 { |
There was a problem hiding this comment.
The value 1000 is a magic number that determines the maximum number of attempts to find a unique filename. To improve code clarity, it's best to define this as a package-level constant.
| candidate := storagePath | ||
| for i := 1; w.Exists(ctx, candidate); i++ { | ||
| candidate = fmt.Sprintf("%s_%d%s", base, i, ext) | ||
| if i > 1000 { |
There was a problem hiding this comment.
The value 1000 is a magic number that determines the maximum number of attempts to find a unique filename. To improve code clarity, it's best to define this as a package-level constant.
2 participants
Summary by Sourcery
Add media group (album) support and enhance storage rule capabilities
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores