COG-412: Migrate javax to jakarta namespace#7
COG-412: Migrate javax to jakarta namespace#7devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Co-Authored-By: patrick.bradley <patrick.bradley@cognition.ai>
- Upgrade Spring Boot parent from 2.2.6.RELEASE to 3.2.5 - Change java.version from 11 to 17 - Add spring-boot-starter-validation dependency - Remove junit-vintage-engine exclusion from spring-boot-starter-test - Replace org.apache.httpcomponents:httpclient with httpclient5 - Replace javax.persistence imports with jakarta.persistence in Bond, Counterparty, Rfq - Replace javax.validation imports with jakarta.validation in Bond, Counterparty, Rfq - Update SplitTheMonolithApplication.java httpclient imports to hc.client5 Co-Authored-By: patrick.bradley <patrick.bradley@cognition.ai>
…vin/1778619585-cog-412-javax-to-jakarta
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
📝 Info: CloseableHttpClient created but never explicitly closed on shutdown
The CloseableHttpClient created at SplitTheMonolithApplication.java:56 is wrapped inside HttpComponentsClientHttpRequestFactory, which implements DisposableBean and would close the client on destruction. However, the factory is not itself a Spring bean — it's created inline and passed to RestTemplate. Since RestTemplate doesn't implement DisposableBean or have a destroy method, the connection pool managed by the HttpClient won't be properly shut down when the Spring context closes. This is a pre-existing issue (same pattern existed before this PR), but it could be improved by making the factory a @Bean or using @PreDestroy to close the client.
(Refers to lines 55-58)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — this is a pre-existing pattern, not introduced by the migration. Noted for a follow-up cleanup.
There was a problem hiding this comment.
📝 Info: @NotNull on primitive long fields is ineffective
The @NotNull annotations on counterpartyId and bondId (both primitive long) at lines 17 and 20 are meaningless — primitives cannot be null and default to 0. If validation were triggered (which it currently isn't since @Valid is missing on the controller method RfqController.java:38), these constraints would always pass. To make @NotNull meaningful, these should be Long (boxed type). This is pre-existing and not introduced by this PR, but the jakarta migration is a good time to fix it.
(Refers to lines 16-20)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pre-existing issue — @NotNull on primitives was already ineffective before the migration. Out of scope for this namespace migration PR.
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; |
There was a problem hiding this comment.
📝 Info: Hibernate 6 changes GenerationType.AUTO to use sequences instead of identity columns
In Hibernate 5 (Spring Boot 2), GenerationType.AUTO with H2 mapped to IDENTITY. In Hibernate 6 (Spring Boot 3), it maps to SEQUENCE, creating sequences like bonds_SEQ, counterparties_SEQ, rfqs_SEQ. Since the app uses H2 in-memory with auto-DDL, this works transparently. The integration tests at monolith/src/test/java/com/javieraviles/splitthemonolith/IntegrationTest.java:170-173 extract IDs dynamically, so they won't break from different starting ID values. However, if there were any hardcoded ID assumptions elsewhere, they could fail. Worth noting for awareness.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good callout. The app uses H2 in-memory with auto-DDL and IDs are extracted dynamically in tests, so this is transparent. Worth noting but no action needed in this PR.
Summary\n\nMigrates the monolith from Spring Boot 2.2.6 (javax namespace) to Spring Boot 3.2.5 (jakarta namespace) as specified in COG-412.\n\nThis is a combined PR merging work from two child sessions:\n- PR #6 — POM upgrade + entity file migrations\n- PR #5 — DTO file migrations\n\n### POM changes (
monolith/pom.xml):\n- Spring Boot parent:2.2.6.RELEASE→3.2.5\n- Java version:11→17\n- Addedspring-boot-starter-validationdependency\n- Removedjunit-vintage-engineexclusion\n- Replacedorg.apache.httpcomponents:httpclientwithorg.apache.httpcomponents.client5:httpclient5\n\n### Java file changes:\n-Bond.java—javax.persistence.*→jakarta.persistence.*,javax.validation.*→jakarta.validation.*\n-Counterparty.java—javax.persistence.*→jakarta.persistence.*,javax.validation.*→jakarta.validation.*\n-Rfq.java—javax.persistence.*→jakarta.persistence.*,javax.validation.*→jakarta.validation.*\n-RfqDto.java—javax.validation.*→jakarta.validation.*\n-TradeConfirmationDto.java—javax.validation.*→jakarta.validation.*\n-SplitTheMonolithApplication.java—org.apache.http→org.apache.hc.client5(httpclient5 migration)\n\n## Review & Testing Checklist for Human\n- [ ] Verify zerojavax.persistenceorjavax.validationimports remain in any Java file\n- [ ] Verifypom.xmltargets Spring Boot 3.2.5 and Java 17\n- [ ] Runcd monolith && ./mvnw clean compilewith JDK 17 to confirm compilation\n- [ ] Runcd monolith && ./mvnw spring-boot:runand test API endpoints (e.g.GET /counterparties,GET /bonds,GET /rfqs)\n\n### Notes\n- Theorders/andnotifications/modules referenced in the ticket do not exist in this repo — onlymonolith/is present\n- Full source tree was searched; no otherjavax.persistenceorjavax.validationreferences remain\n- Build compiles successfully with JDK 17"Link to Devin session: https://app.devin.ai/sessions/c7adac22ed5143c3ad7a5c3d188bce96
Requested by: @patrickbradley-cog
Devin Review