Skip to content

#295 Programmatic validation property path handling #510

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

pazkooda
Copy link
Collaborator

@pazkooda pazkooda commented Jul 4, 2025

@lwitkowski Hey,

I've asked Claude Code to try to address #295 and it came with this solution.
I know it's bit dirty (commits, maybe a bit code structure)

But this Draft PullReq is to discuss if we are OK with the concept itself.

I can do clean-up later on.

@pazkooda pazkooda requested a review from lwitkowski July 4, 2025 12:28
@pazkooda
Copy link
Collaborator Author

pazkooda commented Jul 4, 2025

@bdunni Curious if you could build this branch locally and validate if it works in the case from original issue.

If you could provide a test with validation groups this would be perfect.

pazkooda and others added 6 commits July 4, 2025 15:11
Resolves long-standing issue where ConstraintViolationExceptionMapper incorrectly
stripped method names from property paths in programmatically thrown validation
exceptions.

Changes:
- Implement hybrid validation detection using standard APIs (no deprecated features)
- Differentiate between JAX-RS declarative (@Valid) and programmatic validation
- Preserve original property paths for programmatic validation
- Add comprehensive integration and unit tests for both scenarios
- Remove usage of deprecated ElementKind.METHOD approach

The mapper now:
1. Checks JAX-RS context and parameter matching for declarative validation
2. Examines root bean type and property path structure
3. Defaults to programmatic validation when no JAX-RS context detected
4. Strips method names only for declarative validation cases

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Add programmatic validation endpoints with simple and nested test cases
- Add comprehensive integration tests verifying property path preservation
- Enhance declarative validation tests to ensure proper differentiation
- Test beans for programmatic validation with various constraint types

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Replace v.getField() with v.field since Violation class uses public final fields
instead of getter methods.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Prevent NullPointerException when resourceInfo is null (programmatic validation)
by adding early null checks for both resourceInfo and method.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Cast null to Object to resolve ambiguity between queryParam overloads
in integration test.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Use short name "A" instead of null to trigger @Length constraint,
since RestAssured converts null to string in some profiles.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@pazkooda pazkooda force-pushed the fix/295-programmatic-validation-property-paths branch from 0fa54b7 to d8ea9f6 Compare July 4, 2025 13:13
Comment on lines +151 to +163
if (noJaxRsContext()) {
return false;
}

if (rootBeanResourceClassMatches(violation)) {
return true;
}

if (propertyPathStartsWithMethod(violation)) {
return true;
}

return false;
Copy link
Collaborator Author

@pazkooda pazkooda Jul 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be even simplified to:

return rootBeanResourceClassMatches(violation) || propertyPathStartsWithMethod(violation);

UnitTest from Claude may fail but I think it performs unrealistic scenario. Or maybe code in line 81 will be sufficient to pass even theorethical case.

@lwitkowski
Copy link
Collaborator

@pazkooda Nice! Looks pretty good already, some comments seems a bit redundant, small code cleanups here and there, and we should merge this.

@pazkooda
Copy link
Collaborator Author

pazkooda commented Jul 8, 2025

I just left for vacation (computer-less). Will fix those early next week.

Comments are annoyance of Claude Code, will remove them.

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

Successfully merging this pull request may close these issues.

2 participants