-
Notifications
You must be signed in to change notification settings - Fork 1
Code improvements using Java records 2 #186
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
|
CI: Security warnings found! |
…uplication warnings
|
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 converts several email-related classes from traditional Java classes to Java records, reducing boilerplate code and improving immutability. The changes migrate ExternalRejectEmailData, ExternalRejectEmailModel, InternalAvFailedEmailModel, and InternalAvFailedEmailData to records, along with ExternalAcceptEmailData. All associated tests and usages have been updated to use the new record accessor methods.
Key Changes:
- Converted 5 email model/data classes from traditional classes to Java records
- Updated all accessor method calls from
getXxx()to the record-stylexxx()pattern - Added compact constructors to ensure immutability of list fields using
List.copyOf()
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ExternalRejectEmailData.java | Converted from builder pattern class to record with immutable list handling |
| ExternalRejectEmailModel.java | Converted to record with defensive copying of reject reasons list |
| InternalAvFailedEmailData.java | Converted from builder pattern class to record with immutable list handling |
| InternalAvFailedEmailModel.java | Converted to record with defensive copying of infected files list |
| ExternalAcceptEmailData.java | Converted from builder pattern class to simple record |
| ExternalRejectEmailMapper.java | Updated to use record constructors and accessors, made fields final |
| InternalAvFailedEmailMapper.java | Updated to use record constructors and accessors, made fields final |
| ExternalAcceptEmailMapper.java | Updated to use record constructor instead of builder |
| EmailServiceImpl.java | Updated logging to use record accessors |
| FesServiceImplTest.java | Updated test assertions to use record accessors |
| ExternalRejectEmailDataTest.java | Updated test to construct record directly and use record accessors |
| ExternalRejectEmailMapperTest.java | Updated test to use record constructors and accessors, added final modifiers |
| InternalAVFailedEmailMapperTest.java | Updated test expectations to use record constructors |
| ExternalAcceptEmailMapperTest.java | Updated test to use record constructor, added final modifiers |
| EmailServiceImplTest.java | Updated tests to construct records directly and use record accessors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
CI: Security warnings found! |
PhilipGlover
left a comment
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.
- have run and successfully reach confirmation
- code update look good
mvn verifygreen, pipeline and Sonar pass- most of the code changes are for emails which I haven't checked, since they depend on a response from CHIPS or AWS. This is only possible in cidev or Staging.
Ought the Test notes as well as saying Regression required, to draw attention to Accept, Reject emails? I forget how to test the failed AV check.




Jira ticket: ASM-881
Brief description of the change(s)
Mini PR to cover changes to following classes and their usage:
Working example
Test notes
Regression
Checklist
- [ ] Added/updated logging appropriately.- [ ] Written tests.- [ ] Updated Docker/ECS configs.~~ - [ ] Added all new properties to chs-configs.~~
Strikethrough (
~~like this~~) anything not applicable to your changes.