Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the odata4j project from Java EE to Jakarta EE 9 by updating all JAX-RS and servlet dependencies and configurations. The migration addresses compatibility requirements for modern Java enterprise environments by transitioning from the javax.* namespace to jakarta.*.
Key changes include:
- Updated all JAX-RS imports from javax.ws.rs to jakarta.ws.rs
- Migrated servlet imports from javax.servlet to jakarta.servlet
- Upgraded Jersey from version 1.1.5 to 3.1.3
- Updated Jetty, CXF, and other framework dependencies to Jakarta EE compatible versions
- Modernized Java version from 1.6 to 1.8
Reviewed Changes
Copilot reviewed 122 out of 122 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Multiple test files | Updated imports to use jakarta namespace and modernized test infrastructure |
| Jersey integration files | Completely refactored Jersey client/server code for v3.x compatibility |
| Core producer resources | Updated all JAX-RS annotations and imports to jakarta namespace |
| pom.xml | Updated dependency versions and Java target version |
| Multiple config files | Updated servlet and JAX-RS application configurations |
| import static org.hamcrest.CoreMatchers.notNullValue; | ||
| import static org.junit.Assert.assertThat; | ||
| import static org.junit.Assert.fail; | ||
| import static org.junit.jupiter.api.Assertions.fail; |
There was a problem hiding this comment.
Duplicate import of fail method. Line 6 imports from JUnit Jupiter while line 7 imports from JUnit 4 matchers. This will cause a compilation error due to conflicting imports.
| import static org.junit.jupiter.api.Assertions.fail; |
| import static org.junit.Assert.assertThat; | ||
| import static org.junit.Assert.fail; | ||
| import static org.junit.jupiter.api.Assertions.fail; | ||
| import static org.junit.matchers.JUnitMatchers.containsString; |
There was a problem hiding this comment.
The import org.junit.matchers.JUnitMatchers.containsString is deprecated. This should be replaced with org.hamcrest.Matchers.containsString for modern JUnit versions.
| import static org.junit.matchers.JUnitMatchers.containsString; | |
| import static org.hamcrest.Matchers.containsString; |
| try { | ||
| unauthorizedConsumer.getEntities("Persons").execute(); | ||
| fail(); | ||
| Assert.fail(); |
There was a problem hiding this comment.
Using Assert.fail() when fail() is already imported. This is inconsistent with the static import on line 5 and should use the imported method instead.
| Assert.fail(); | |
| fail(); |
| ContentExchange exchange = sendRequest(BASE_URI); | ||
| exchange.waitForDone(); | ||
| ContentResponse response = sendRequest(BASE_URI); | ||
| // exchange.waitForDone(); |
There was a problem hiding this comment.
Dead code comment should be removed. The commented line refers to old Jetty API that is no longer needed in the updated implementation.
| // exchange.waitForDone(); |
| } catch (IllegalAccessException e) { | ||
| throw Throwables.propagate(e); | ||
| } | ||
| // try { |
There was a problem hiding this comment.
Large block of commented-out code (lines 146-181) should be removed rather than left as comments. This appears to be old implementation code that has been replaced.
| return entity; | ||
| } | ||
|
|
||
| // /** |
There was a problem hiding this comment.
Large block of commented-out code (lines 104-241) for batch request handling should be removed if not needed, or completed if this functionality is required.
| import org.glassfish.jersey.client.JerseyClient; | ||
| import org.glassfish.jersey.client.JerseyClientBuilder; | ||
|
|
||
| import jakarta.ws.rs.client.Client; | ||
| import org.glassfish.jersey.client.ClientConfig; | ||
| import org.glassfish.jersey.client.JerseyClient; | ||
| import org.glassfish.jersey.client.JerseyClientBuilder; | ||
|
|
There was a problem hiding this comment.
Duplicate imports: JerseyClient and JerseyClientBuilder are imported twice (lines 3-4 and 8-9). Remove the duplicate imports on lines 3-4.
| import org.glassfish.jersey.client.JerseyClient; | |
| import org.glassfish.jersey.client.JerseyClientBuilder; | |
| import jakarta.ws.rs.client.Client; | |
| import org.glassfish.jersey.client.ClientConfig; | |
| import org.glassfish.jersey.client.JerseyClient; | |
| import org.glassfish.jersey.client.JerseyClientBuilder; | |
| import jakarta.ws.rs.client.Client; | |
| import org.glassfish.jersey.client.ClientConfig; | |
| import org.glassfish.jersey.client.JerseyClient; | |
| import org.glassfish.jersey.client.JerseyClientBuilder; | |
| import junit.framework.Assert; | ||
|
|
||
| import org.joda.time.LocalDateTime; |
There was a problem hiding this comment.
Using deprecated JUnit 3 Assert class. Should use org.junit.Assert instead of junit.framework.Assert for consistency with modern JUnit practices.
| import junit.framework.Assert; | |
| import org.joda.time.LocalDateTime; | |
| import org.junit.Assert; | |
| import org.joda.time.LocalDateTime; |
1 participant
No description provided.