Skip to content

COG-412: Migrate javax to jakarta namespace#7

Open
devin-ai-integration[bot] wants to merge 3 commits into
masterfrom
devin/1778619585-cog-412-javax-to-jakarta
Open

COG-412: Migrate javax to jakarta namespace#7
devin-ai-integration[bot] wants to merge 3 commits into
masterfrom
devin/1778619585-cog-412-javax-to-jakarta

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented May 12, 2026

Copy link
Copy Markdown

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.RELEASE3.2.5\n- Java version: 1117\n- Added spring-boot-starter-validation dependency\n- Removed junit-vintage-engine exclusion\n- Replaced org.apache.httpcomponents:httpclient with org.apache.httpcomponents.client5:httpclient5\n\n### Java file changes:\n- Bond.javajavax.persistence.*jakarta.persistence.*, javax.validation.*jakarta.validation.*\n- Counterparty.javajavax.persistence.*jakarta.persistence.*, javax.validation.*jakarta.validation.*\n- Rfq.javajavax.persistence.*jakarta.persistence.*, javax.validation.*jakarta.validation.*\n- RfqDto.javajavax.validation.*jakarta.validation.*\n- TradeConfirmationDto.javajavax.validation.*jakarta.validation.*\n- SplitTheMonolithApplication.javaorg.apache.httporg.apache.hc.client5 (httpclient5 migration)\n\n## Review & Testing Checklist for Human\n- [ ] Verify zero javax.persistence or javax.validation imports remain in any Java file\n- [ ] Verify pom.xml targets Spring Boot 3.2.5 and Java 17\n- [ ] Run cd monolith && ./mvnw clean compile with JDK 17 to confirm compilation\n- [ ] Run cd monolith && ./mvnw spring-boot:run and test API endpoints (e.g. GET /counterparties, GET /bonds, GET /rfqs)\n\n### Notes\n- The orders/ and notifications/ modules referenced in the ticket do not exist in this repo — only monolith/ is present\n- Full source tree was searched; no other javax.persistence or javax.validation references remain\n- Build compiles successfully with JDK 17"

Link to Devin session: https://app.devin.ai/sessions/c7adac22ed5143c3ad7a5c3d188bce96
Requested by: @patrickbradley-cog


Devin Review

Status Commit
⚪ Not started

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

Open in Devin Review (Staging)
Open in Devin Review

devin-ai-integration Bot and others added 3 commits May 12, 2026 20:54
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>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

Open in Devin Review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 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)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Acknowledged — this is a pre-existing pattern, not introduced by the migration. Noted for a follow-up cleanup.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 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)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Pre-existing issue — @NotNull on primitives was already ineffective before the migration. Out of scope for this namespace migration PR.

Comment on lines +9 to +10
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

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.

1 participant