Update java sdk for version 3 release #71
Conversation
Make changes in the sdk to accept the new class Test changes Enable full javac linting and resolve legacy warnings upgrade maven-compiler-plugin to 3.13.0, compile with --release 8, and turn on -Xlint:all with targeted suppressions add serialVersionUID, swap Jackson currentName() usage, refine retry policy constructors, and avoid deprecated SSL provider APIs Add script and pom to run validation and then fix warnings Added static analysis checks Add more rules for static analysis Fix tests
There was a problem hiding this comment.
Pull Request Overview
Modernizes the Java SDK for version 3.0.0 by replacing the deprecated OAuth2 module with Azure Identity library support, fixing documentation issues, and cleaning up tests to address linter warnings.
- Replaces custom OAuth implementation with Azure Identity
TokenCredential - Updates dependencies including Jackson, Maven plugins, and testing libraries
- Removes incomplete/disabled test methods and improves test performance with parallel file creation
Reviewed Changes
Copilot reviewed 51 out of 52 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ADLStoreClient.java | Updated factory methods to accept TokenCredential instead of custom OAuth classes |
| HttpTransport.java | Added request/response logging and modernized URL creation |
| Test files | Removed incomplete tests, fixed assertion bugs, and added parallel file creation |
| oauth2/ package | Removed entire deprecated OAuth implementation |
| pom.xml | Updated Maven plugins, Jackson, and added Azure Identity dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| assertTrue("Total time was more than max duration expected", duration < expectedDurationMax); | ||
| assertTrue("Total time was less than min duration expected", duration > expectedDurationMin); |
There was a problem hiding this comment.
The assertion messages are incorrect. Line 85 should state 'less than max duration' and line 86 should state 'more than min duration' to match the actual conditions being tested.
| assertTrue("Total time was more than max duration expected", duration < expectedDurationMax); | |
| assertTrue("Total time was less than min duration expected", duration > expectedDurationMin); | |
| assertTrue("Total time was less than max duration expected", duration < expectedDurationMax); | |
| assertTrue("Total time was more than min duration expected", duration > expectedDurationMin); |
| try { | ||
| url = new URL(urlString.toString()); | ||
| url = URI.create(urlString.toString()).toURL(); | ||
| } catch (MalformedURLException ex) { |
There was a problem hiding this comment.
Creating a URI from a StringBuilder's toString() and then converting to URL can throw IllegalArgumentException for malformed URIs. The original URL constructor with proper exception handling should be preserved, or the URI creation should be wrapped in a try-catch for IllegalArgumentException.
| } catch (MalformedURLException ex) { | |
| } catch (IllegalArgumentException | MalformedURLException ex) { |
| while (jp.hasCurrentToken()) { | ||
| if (jp.getCurrentToken() == JsonToken.FIELD_NAME) { | ||
| fieldName = jp.getCurrentName(); | ||
| fieldName = jp.currentName(); |
There was a problem hiding this comment.
The method currentName() should be getCurrentName() for Jackson compatibility. The deprecated method may not exist in newer Jackson versions.
| fieldName = jp.currentName(); | |
| fieldName = jp.getCurrentName(); |
| while (jp.hasCurrentToken()) { | ||
| if (jp.getCurrentToken() == JsonToken.FIELD_NAME) { | ||
| fieldName = jp.getCurrentName(); | ||
| fieldName = jp.currentName(); |
There was a problem hiding this comment.
The method currentName() should be getCurrentName() for Jackson compatibility. The deprecated method may not exist in newer Jackson versions used in the updated dependencies.
| fieldName = jp.currentName(); | |
| fieldName = jp.getCurrentName(); |
| while (jp.hasCurrentToken()) { | ||
| if (jp.getCurrentToken() == JsonToken.FIELD_NAME) { | ||
| fieldName = jp.getCurrentName(); | ||
| fieldName = jp.currentName(); |
There was a problem hiding this comment.
The method currentName() should be getCurrentName() for Jackson compatibility. The deprecated method may not exist in newer Jackson versions used in the updated dependencies.
| fieldName = jp.currentName(); | |
| fieldName = jp.getCurrentName(); |
| } | ||
| if (jp.getCurrentToken() == JsonToken.FIELD_NAME) { | ||
| fieldName = jp.getCurrentName(); | ||
| fieldName = jp.currentName(); |
There was a problem hiding this comment.
The method currentName() should be getCurrentName() for Jackson compatibility. The deprecated method may not exist in newer Jackson versions used in the updated dependencies.
| fieldName = jp.currentName(); | |
| fieldName = jp.getCurrentName(); |
| while (jp.hasCurrentToken()) { | ||
| if (jp.getCurrentToken() == JsonToken.FIELD_NAME) { | ||
| fieldName = jp.getCurrentName(); | ||
| fieldName = jp.currentName(); |
There was a problem hiding this comment.
The method currentName() should be getCurrentName() for Jackson compatibility. The deprecated method may not exist in newer Jackson versions used in the updated dependencies.
| fieldName = jp.currentName(); | |
| fieldName = jp.getCurrentName(); |
| if (linearRetryInterval < 0) { | ||
| throw new IllegalArgumentException("linearRetryInterval must be non-negative."); | ||
| } |
There was a problem hiding this comment.
The validation checks linearRetryInterval but the parameter is documented as ignored for backwards compatibility. This validation should be removed since the parameter has no functional impact.
| if (linearRetryInterval < 0) { | |
| throw new IllegalArgumentException("linearRetryInterval must be non-negative."); | |
| } |
Remove dependency on oauth custom code and use azure library
Fix doc string errors.
Cleanup tests and various linter errors.