Skip to content

Added validation functionality when reading files, generating error messages based on validation results. Supports manually adding error messages #282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Added validation functionality when reading files, generating error messages based on validation results. Supports manually adding error messages #282

wants to merge 3 commits into from

Conversation

wangmiscoding
Copy link
Contributor

Added validation functionality when reading files, generating error messages based on validation results. Supports manually adding error messages. Two methods for handling error messages were introduced:

Writing error messages back into the Excel file.
Concatenating error messages into a text format.

enhancement:validate when import
6c8fe25 
- Added ValidateReadListener to record validation error messages.
- Introduced ValidateError and ValidateErrorHolder to store error messages.
- Added FileErrorHandler and TextErrorHandler to handle error messages.
- Updated ExcelReaderBuilder to include validation-related methods.
- Added test cases to verify the validation feature.
@wangmiscoding
Copy link
Contributor Author

@psxjoy This update adds the ability to validate whether a field is empty using the @ExcelProperty annotation. Future improvements may include length validation, duplicate checking, and more.

Current Issues & Considerations:
When reading a file, if there is an error in the content, an ExcelDataConvertException is thrown, causing the program to terminate. This means error messages can only be collected up to that point.
To prevent termination, users must manually handle exceptions using try-catch.
Two error-handling strategies are defined: FileErrorHandler (writes errors back to Excel) and TextErrorHandler (concatenates errors into text). Users can also define custom handlers.
Request for Feedback:
Are there any issues with my implementation?
Is my overall approach feasible?
Any suggestions for improvement?
I appreciate any insights and guidance from experienced developers!

fix: Fix issue where error files were not deleted
c505d8a 
Added errorFile.deleteOnExit() after creating the error file directory.
Ensured that the generated error files are deleted when the program exits
@wangmiscoding
Copy link
Contributor Author

@psxjoy I checked the error message, and theoretically, the folder should be created when the FileUtils class is loaded. Could you help me check this?
image

@psxjoy
Copy link
Member

psxjoy commented Mar 22, 2025

All test cases are expected to pass. In my opinion, there might be an issue in your logic causing the tests to fail. You might want to debug it locally to identify the problem.
所有的测试用例都应该通过的。我认为是你新增的一些逻辑导致测试用例报错。或许你可以本地调试一下。

@wangmiscoding
Copy link
Contributor Author

All test cases are expected to pass. In my opinion, there might be an issue in your logic causing the tests to fail. You might want to debug it locally to identify the problem.
我在我的本地没有复现，我在这个地方添加了创建临时目录的代码，有没有可能流水线执行的时候没有创建这个目录的权限
{8A6643F3-CC66-45F4-A0EF-0AE5AC308B28}

@psxjoy
Copy link
Member

psxjoy commented Mar 22, 2025

This is a major change, so I’ll take some time to think it through and also consider feedback from others during the code review.

这是个大的改动，我需要考虑一下并且参考其他的人code review建议。

@psxjoy psxjoy added the pending verification This problem needs to be confirmed label Mar 22, 2025
@wangmiscoding
Copy link
Contributor Author

This is a major change, so I’ll take some time to think it through and also consider feedback from others during the code review.这是一个重大更改，因此我将花一些时间仔细考虑，并在代码审查期间考虑其他人的反馈。

这是个大的改动，我需要考虑一下并且参考其他的人code review建议。

Okay, my design may have many issues. Please let me know if you have any feasible suggestions. Thanks!

@wangmiscoding wangmiscoding mentioned this pull request Mar 24, 2025
Open
@wangmiscoding
Copy link
Contributor Author

#282

Merge branch 'main' of https://github.com/wangmiscoding/fastexcel int…
3a5ccad 
…o feature/import-validate

# Conflicts:
#	fastexcel-core/src/main/java/cn/idev/excel/annotation/ExcelProperty.java
#	fastexcel-core/src/main/java/cn/idev/excel/read/builder/ExcelReaderBuilder.java
#	fastexcel-core/src/main/java/cn/idev/excel/util/FileUtils.java
@wangmiscoding
Copy link
Contributor Author

使用效果：
image

@psxjoy
Copy link
Member

psxjoy commented Mar 31, 2025

This change is indeed impactful. However, since no one else has reviewed it yet, I'll temporarily mark it as develop completed. This PR will be merged into the main branch after receiving at least +3 (bonding) votes. Thanks for your contribution!

这个改动的确很有作用。但是目前没人其他人code reveiw，我暂时把他标记为 develop completed。该PR 会在至少 +3(bonding)投票后合并到main年分支。感谢你的贡献！

@psxjoy psxjoy added the development completed Development completed, waiting for release label Mar 31, 2025
@wangmiscoding
Copy link
Contributor Author

This change is indeed impactful. However, since no one else has reviewed it yet, I'll temporarily mark it as develop completed. This PR will be merged into the main branch after receiving at least +3 (bonding) votes. Thanks for your contribution!

这个改动的确很有作用。但是目前没人其他人code reveiw，我暂时把他标记为 develop completed。该PR 会在至少 +3(bonding)投票后合并到main年分支。感谢你的贡献！

Thank you! If there's anything I can do, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jipengfei-jpf jipengfei-jpf Awaiting requested review from jipengfei-jpf jipengfei-jpf is a code owner

@psxjoy psxjoy Awaiting requested review from psxjoy psxjoy is a code owner

Assignees
No one assigned
Labels
development completed Development completed, waiting for release pending verification This problem needs to be confirmed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wangmiscoding @psxjoy