Skip to content
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.

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

Validation reason is incorrectly called FAILED_CUSTOM_VALIDATION #182

Open
1 task
tbatchlear opened this issue Oct 18, 2023 · 1 comment
Open
1 task
Assignees
Labels
enhancement New feature or request

Comments

@tbatchlear
Copy link

tbatchlear commented Oct 18, 2023

Is this bug report about an incorrectly validated email address?

  • Yes
  • [ X ] No

If no, please describe the bug

When validating an email address in the following manner, the validation failure message is incorrect:

String invalidDomain = "my.email@example.org";
var result  = javaJMail.validator().requireOnlyTopLevelDomain(TopLevelDomain.DOT_COM).validate(invalidDomain);

This returns a FailureReason.FAILED_CUSTOM_VALIDATION. There should be a failure reason to match the validation here done on this domain. Perhaps a FailureReason.INVALID_TOP_LEVEL_DONAIN would be appropriate.

Additionally, if passing my.email@example to the above validation, it still returns the FAILED_CUSTOM_VALIDATION reason. However, there is already an existing "MISSING_TOP_LEVEL_DOMAIN" that should be used here instead when used in conjunction with the requireOnlyTopLevelDomain(), as this implies that a TLD is required.

Finally, as a side thought on this, it would be nice to be able to add a custom failure reason, perhaps along with the custom validator?

public EmailValidator withRule(Predicate<Email> rule, String failureReason);

Then, instead of these predicates being stored as a Set, they could be stored as a Map along with their failure reason.

@tbatchlear tbatchlear added the bug Something isn't working label Oct 18, 2023
@RohanNagar RohanNagar added enhancement New feature or request and removed bug Something isn't working labels Oct 18, 2023
@RohanNagar
Copy link
Owner

Hi @tbatchlear, thank you so much for opening this issue!

You are correct that the failure reasons when adding additional rules are not very great at the moment. We also have issue #140 open to address this.

I've actually already started the work for these improvements, including:

  1. Better failure reasons for built-in rules like requireOnlyTopLevelDomain
  2. The option to add your own custom FailureReason when adding a custom rule

I'll keep this open as it is a great motivator when multiple people are requesting a feature 🙂. I hope to have this ready for you soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants