akshat-lakhera opened a new pull request, #20688:
URL: https://github.com/apache/druid/pull/20688
Part of #20525.
### Description
This is the first cleanup from the table in #20525. It replaces the
deprecated java.net.URL constructors in production code with
URI.create(...).toURL() or new URI(...).toURL(). Test code still uses the old
constructors, so I didn't add the forbidden-apis rule here. I plan to clean up
the tests and add the rule in a follow-up PR.
I tried to keep behaviour the same:
- Where the old code turned MalformedURLException into a specific exception
(ISE in HttpPostEmitter, ParseException in FileBasedProtobufBytesDecoder, IAE
in HttpServerInventoryView), the catch now also covers IllegalArgumentException
or URISyntaxException, so callers see the same exception type.
- For pre-encoded paths (TaskLocation, ServiceLocation,
LookupCoordinatorManager) I build the URI from a string with Guava HostAndPort
rather than the multi-arg URI constructors, which would encode the path a
second time. HostAndPort also keeps IPv6 hosts in brackets like the old URL
constructor did, and the port is left out when it isn't set.
- Relative forms (HttpLoadQueuePeon, ChangeRequestHttpSyncer) now use
URI.resolve. I checked it gives the same result as the old new URL(base, spec)
for bases with no path, a trailing slash, or one path segment.
- URI is stricter than URL, so malformed input such as unencoded spaces is
now rejected earlier.
I built the touched modules and ran the unit tests for the changed classes.
They pass, except DirectDruidClientTest and
FileBasedProtobufBytesDecoderTest.testDescriptorUrl on my Windows machine: the
first fails in setup on temp file locking, and the second builds a file URL
from a Windows path with backslashes, which URI rejects. Both should be fine on
Linux CI.
#### Release note
No release note, there's no user-facing change.
<hr>
##### Key changed/added classes in this PR
* TaskLocation
* ServiceLocation
* HttpPostEmitter
* ChannelResourceFactory
* DirectDruidClient
* HttpServerInventoryView
* HttpLoadQueuePeon
* ChangeRequestHttpSyncer
* LookupCoordinatorManager
* BuildInfo
* CoordinatorRedirectInfo
* OverlordRedirectInfo
* SystemServerPropertiesTable
* PrometheusEmitter
* CommonCacheNotifier
* RestUpdateSender
* FileBasedProtobufBytesDecoder
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [x] a behaviour-preserving refactor covered by the existing unit tests of
the changed classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
