refactor: use timestamp+filename for bulk message request ID#903
Conversation
- Generate request ID as bulk-{base62_timestamp}-{truncated_filename}
- Encode unix timestamp as base62 for minimal length (~6 chars)
- Truncate filename to max 32 chars preserving extension
- Remove fileType return value from ValidateStore
- Simplify frontend cleanName() to just strip 'bulk-' prefix
- Stay on bulk-messages page after upload instead of redirecting
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 4 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Greptile SummaryThis PR changes how bulk message request IDs are generated — from
Confidence Score: 3/5The core ID-generation change introduces a real collision window: two uploads of the same filename within the same second produce identical request IDs, merging their message batches in the database and the history view. The timestamp has only second-level precision, so rapid or concurrent uploads of identically-named files silently share a request ID — every message from both batches becomes indistinguishable. Raw user-supplied filenames are also embedded without sanitization, and the simplified cleanName function breaks display of all pre-existing database entries. Focus on api/pkg/handlers/bulk_message_handler.go — specifically the generateRequestID function and its lack of a uniqueness guarantee — and web/pages/bulk-messages/index.vue for the backward-compatibility gap in cleanName. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (Browser)
participant V as index.vue
participant A as BulkMessageHandler
participant Val as BulkMessageHandlerValidator
participant S as MessageService
U->>V: Upload CSV/XLSX file
V->>A: POST /v1/bulk-messages (multipart)
A->>Val: ValidateStore(ctx, userID, file)
Val-->>A: messages, userLocation, errors
A->>A: generateRequestID(file.Filename)
note over A: bulk-{base62(now.Unix())}-{truncate(filename,32)}
loop For each message (goroutine)
A->>S: SendMessage(requestID, ...)
end
A-->>V: 202 Accepted
V->>V: "formFile = null"
V->>A: GET /v1/bulk-messages
A-->>V: bulk order history
V-->>U: Refresh history table
Reviews (1): Last reviewed commit: "refactor: use timestamp+filename for bul..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func (h *BulkMessageHandler) generateRequestID(filename string) string { | ||
| timestamp := encodeBase62(time.Now().Unix()) | ||
| truncated := truncateFilename(filename, 32) | ||
| return fmt.Sprintf("bulk-%s-%s", timestamp, truncated) | ||
| } |
There was a problem hiding this comment.
time.Now().Unix() has one-second granularity, so two uploads of the same filename that arrive within the same second produce an identical requestID. All messages from both batches would be tagged with the same ID and become indistinguishable in search and in the history table. The previous nanoid-based scheme avoided this by adding 10 random characters. Adding even 4 random alphanumeric characters after the timestamp (e.g. bulk-{ts}-{file}-{rand4}) would make collisions astronomically unlikely while keeping the human-readable filename benefit.
There was a problem hiding this comment.
Fixed. Added a 4-character random base62 suffix after the timestamp, making the format \�ulk-{base62_ts}{rand4}-{filename}. This makes same-second collisions astronomically unlikely while keeping the human-readable filename.
| cleanName(requestId: string): string { | ||
| if (requestId.startsWith('bulk-csv-')) { | ||
| return requestId.replace(/^bulk-csv-/, '') + '.csv' | ||
| } | ||
| if (requestId.startsWith('bulk-xls-')) { | ||
| return requestId.replace(/^bulk-xls-/, '') + '.xlsx' | ||
| } | ||
| return requestId.replace(/^bulk-/, '') | ||
| }, |
There was a problem hiding this comment.
cleanName breaks display of historical entries
The simplified regex strips only the bulk- prefix, so existing database entries with the old bulk-csv-NANOID or bulk-xls-NANOID format will now render as csv-NANOID / xls-NANOID in the history table. Previously they rendered as NANOID.csv / NANOID.xlsx. A backward-compatible implementation would retain the old prefix checks as fallback branches for entries that don't match the new bulk-{ts}-{filename} pattern.
There was a problem hiding this comment.
Fixed. Restored the old prefix checks as fallback branches for legacy entries (bulk-csv-/bulk-xls-). New entries using the timestamp format just strip the bulk- prefix.
| func truncateFilename(filename string, maxLen int) string { | ||
| if len(filename) <= maxLen { | ||
| return filename | ||
| } | ||
| for i := range b { | ||
| b[i] = alphabet[int(b[i])%len(alphabet)] | ||
| ext := filepath.Ext(filename) | ||
| name := filename[:len(filename)-len(ext)] | ||
| available := maxLen - len(ext) | ||
| if available <= 0 { | ||
| return filename[:maxLen] | ||
| } | ||
| return string(b) | ||
| half := available / 2 | ||
| return name[:half] + name[len(name)-(available-half):] + ext | ||
| } |
There was a problem hiding this comment.
truncateFilename preserves the raw filename characters (spaces, Unicode, dashes, dots, %, etc.) from the client's upload. The previous scheme used only a controlled string ("csv" or "xls"). If requestID is ever used in a log query, a URL parameter, or any downstream context that doesn't expect arbitrary text, the unsanitized portion could cause unexpected behavior. Consider stripping or replacing non-alphanumeric characters (excluding . and -) before embedding the filename in the ID.
There was a problem hiding this comment.
Fixed. Added a sanitizeFilename function that strips all characters except alphanumeric, dots, and hyphens before embedding the filename in the request ID.
- Add 4-char random base62 suffix to prevent same-second collisions - Sanitize filename by stripping non-alphanumeric chars (except . and -) - Restore backward-compat in cleanName for old bulk-csv-/bulk-xls- entries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 participant
Changes