-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore(validation): rework dto validation layer #66
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
Conversation
a61daaa to
09617cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Jakarta Bean Validation annotations to DTOs and implements comprehensive validation test coverage. The changes introduce field-level validation constraints (e.g., @NotBlank, @Positive, @PositiveOrZero) to ensure data integrity at the API layer.
- Added Jakarta Validation and Hibernate Validator dependencies
- Annotated DTO fields with validation constraints (
@NotBlank,@NotEmpty,@Positive,@PositiveOrZero,@Nullable) - Created a base validation test class and comprehensive validation tests for all DTOs
- Fixed field naming inconsistency (
description→commentin NotificationModelDTO)
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle.kts | Added version definitions for hibernate-validator (9.0.1.Final) and jakarta.validation-api (3.1.1) |
| build.gradle.kts | Added validation library dependencies to testImplementation and reordered testRuntimeOnly |
| ValidationTestBase.java | Created abstract base class with helper methods for validation testing using Jakarta Validator |
| SoundFileSourceDTO.java | Added validation constraints for name, volume, pitch, weight, attenuationDistance, and type fields |
| SoundEventDTO.java | Added @notblank and @NotNull constraints to uiName, variableName, keyName, and subTitle fields |
| NotificationModelDTO.java | Added validation constraints and renamed field from "description" to "comment" |
| ItemModelDTO.java | Added validation constraints for string fields and numeric fields (customModelData, amount) |
| FontModelDTO.java | Added validation constraints for string fields, marked comment as @nullable, added @PositiveOrZero for ascent/height |
| AttributeModelDTO.java | Added validation constraints for string fields and numeric constraints for defaultValue and maximumValue |
| SoundFileSourceDTOValidationTest.java | Created comprehensive tests validating blank/negative constraints on all fields |
| SoundEventDTOValidationTest.java | Created tests validating blank string constraints on all required fields |
| NotificationModelDTOValidationTest.java | Created tests validating blank string constraints on all required fields |
| ItemModelDTOValidationTest.java | Created tests validating blank strings, negative numbers, and comment nullability |
| FontModelDTOValidationTest.java | Created tests validating blank strings and negative numeric values |
| AttributeModelDTOValidationTest.java | Created tests validating empty strings and numeric constraints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/net/onelitefeather/vulpes/backend/domain/sound/SoundFileSourceDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/net/onelitefeather/vulpes/backend/domain/sound/SoundFileSourceDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/net/onelitefeather/vulpes/backend/domain/sound/SoundEventDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/net/onelitefeather/vulpes/backend/domain/notification/NotificationModelDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/net/onelitefeather/vulpes/backend/domain/notification/NotificationModelDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/net/onelitefeather/vulpes/backend/domain/font/FontModelDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/net/onelitefeather/vulpes/backend/domain/attribute/AttributeModelDTO.java
Outdated
Show resolved
Hide resolved
src/test/java/net/onelitefeather/vulpes/backend/domain/ValidationTestBase.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Proposed changes
Recent commits removed the existing validation layer for DTOs. While this is acceptable for now, it’s not sustainable in the long run. This pull request introduces a new validation layer that works better for each case
Types of changes
What types of changes does your code introduce to this project?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any ofthem, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before
merging your code.