migrate: hustoj: fix bugs - #1013
Conversation
WalkthroughThe script in Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/migrate/scripts/hustoj.tsOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/migrate/scripts/hustoj.ts(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/migrate/scripts/hustoj.ts (1)
packages/utils/lib/common.ts (1)
noop(153-153)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/migrate/scripts/hustoj.ts (2)
196-206: Good improvement to file path handling!The enhanced regex pattern and file path construction correctly handles both
/upload/aa/xyz.jpgand/upload/aa/bb/xyz.jpgformats. The use offixFileNameensures consistent filename normalization across the migration.
288-291: Excellent security enhancement for private contests!Automatically generating secure passwords for private contests that lack them prevents unauthorized access while maintaining the contest's private nature. The 32-character hex password provides strong security.
| function fixFileName(fileName: string) { | ||
| if (fileName.endsWith('.in') || fileName.endsWith('.out')) { | ||
| const dotAt = fileName.lastIndexOf('.'); | ||
| const name = fileName.slice(0, dotAt); | ||
| const suffix = fileName.slice(dotAt + 1); | ||
| if (!name.match(/[0-9]/)) { | ||
| fileName = `${name}0.${suffix}`; | ||
| } | ||
| } | ||
| return fileName.replace(/[\\/?#~!|*]/g, '_'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance special character replacement for better cross-platform compatibility.
The current regex pattern might miss some problematic characters that could cause issues on different platforms. Consider expanding it to include Windows reserved characters and other potentially problematic ones.
function fixFileName(fileName: string) {
+ if (!fileName) return fileName;
if (fileName.endsWith('.in') || fileName.endsWith('.out')) {
const dotAt = fileName.lastIndexOf('.');
const name = fileName.slice(0, dotAt);
const suffix = fileName.slice(dotAt + 1);
if (!name.match(/[0-9]/)) {
fileName = `${name}0.${suffix}`;
}
}
- return fileName.replace(/[\\/?#~!|*]/g, '_');
+ // Replace Windows reserved characters and other problematic ones
+ return fileName.replace(/[<>:"/\\|?*#~!]/g, '_');
}📝 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.
| function fixFileName(fileName: string) { | |
| if (fileName.endsWith('.in') || fileName.endsWith('.out')) { | |
| const dotAt = fileName.lastIndexOf('.'); | |
| const name = fileName.slice(0, dotAt); | |
| const suffix = fileName.slice(dotAt + 1); | |
| if (!name.match(/[0-9]/)) { | |
| fileName = `${name}0.${suffix}`; | |
| } | |
| } | |
| return fileName.replace(/[\\/?#~!|*]/g, '_'); | |
| } | |
| function fixFileName(fileName: string) { | |
| if (!fileName) return fileName; | |
| if (fileName.endsWith('.in') || fileName.endsWith('.out')) { | |
| const dotAt = fileName.lastIndexOf('.'); | |
| const name = fileName.slice(0, dotAt); | |
| const suffix = fileName.slice(dotAt + 1); | |
| if (!name.match(/[0-9]/)) { | |
| fileName = `${name}0.${suffix}`; | |
| } | |
| } | |
| // Replace Windows reserved characters and other problematic ones | |
| return fileName.replace(/[<>:"/\\|?*#~!]/g, '_'); | |
| } |
🤖 Prompt for AI Agents
In packages/migrate/scripts/hustoj.ts around lines 58 to 68, the regex used to
replace special characters in file names is too limited and may miss characters
that cause issues on some platforms. Update the regex pattern in the replace
method to include a broader set of reserved and problematic characters, such as
Windows reserved characters like < > : " / \ | ? * and others, to ensure better
cross-platform compatibility.
| const allowedUser:{ user_id:string }[] = await query(`SELECT * FROM privilege WHERE rightstr = 'c${tdoc.contest_id}';`); | ||
| for (let i = 0; i < allowedUser.length; i++) { | ||
| await ContestModel.attend(domainId, tid, uidMap[allowedUser[i].user_id]).catch(noop); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation and improve error handling for contest enrollment.
The current implementation silently ignores errors which could hide important issues like missing users or enrollment failures. Consider validating user existence and logging errors for debugging.
const allowedUser:{ user_id:string }[] = await query(`SELECT * FROM privilege WHERE rightstr = 'c${tdoc.contest_id}';`);
for (let i = 0; i < allowedUser.length; i++) {
- await ContestModel.attend(domainId, tid, uidMap[allowedUser[i].user_id]).catch(noop);
+ const uid = uidMap[allowedUser[i].user_id];
+ if (!uid) {
+ report({ message: `warning: user ${allowedUser[i].user_id} not found for contest ${tdoc.contest_id}` });
+ continue;
+ }
+ await ContestModel.attend(domainId, tid, uid).catch((err) => {
+ report({ message: `warning: failed to enroll user ${allowedUser[i].user_id} in contest ${tdoc.contest_id}: ${err.message}` });
+ });
}📝 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.
| const allowedUser:{ user_id:string }[] = await query(`SELECT * FROM privilege WHERE rightstr = 'c${tdoc.contest_id}';`); | |
| for (let i = 0; i < allowedUser.length; i++) { | |
| await ContestModel.attend(domainId, tid, uidMap[allowedUser[i].user_id]).catch(noop); | |
| } | |
| const allowedUser:{ user_id:string }[] = await query(`SELECT * FROM privilege WHERE rightstr = 'c${tdoc.contest_id}';`); | |
| for (let i = 0; i < allowedUser.length; i++) { | |
| const uid = uidMap[allowedUser[i].user_id]; | |
| if (!uid) { | |
| report({ message: `warning: user ${allowedUser[i].user_id} not found for contest ${tdoc.contest_id}` }); | |
| continue; | |
| } | |
| await ContestModel.attend(domainId, tid, uid).catch((err) => { | |
| report({ message: `warning: failed to enroll user ${allowedUser[i].user_id} in contest ${tdoc.contest_id}: ${err.message}` }); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/migrate/scripts/hustoj.ts around lines 301 to 304, the code
currently ignores errors during contest enrollment by using catch(noop), which
can hide issues. Modify the loop to first validate that each user exists before
attempting enrollment, and replace the silent catch with proper error handling
that logs any errors encountered during ContestModel.attend calls to aid
debugging.
2 participants
/upload/aa/xyz.jpg, but some file's format is/upload/aa/bb/xyz.jpg)Summary by CodeRabbit
New Features
Improvements