Feature/issue 1323 inetutils support#4600
Feature/issue 1323 inetutils support#4600myProjectsRavi wants to merge 3 commits intocodecentric:masterfrom
Conversation
erikpetzold
left a comment
erikpetzold
left a comment
There was a problem hiding this comment.
I did not yet have a look at the changes in detail yet, but please clean up the PR
| @@ -0,0 +1,54 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <project xmlns="http://maven.apache.org/POM/4.0.0" | |||
There was a problem hiding this comment.
I think all these oauth2 sample files do not belong here
| */ | ||
| public class DefaultApplicationFactory implements ApplicationFactory { | ||
|
|
||
| // Removed InetUtils dependency; using standard Java APIs for host resolution. |
There was a problem hiding this comment.
there was no InetUtils dependency removed here, please remove the comment
| return address.getHostName(); | ||
| default: | ||
| return address.getCanonicalHostName(); | ||
| } |
There was a problem hiding this comment.
why did you change the syntax here?
|
|
||
| private final MetadataContributor metadataContributor; | ||
|
|
||
| private final Optional<?> inetUtils; |
There was a problem hiding this comment.
I'm not one of the contributors of this project but just a client of it, but I must say that this <?> really smells of bad code and potential broken injection points...
| try { | ||
| java.lang.reflect.Method m = utils.getClass().getMethod("findFirstNonLoopbackHostInfo"); | ||
| Object host = m.invoke(utils); | ||
| if (host instanceof String && StringUtils.hasText((String) host)) { |
There was a problem hiding this comment.
It can probably be simplified with pattern matching for instanceof if the Java version of the project allows it ;)
| if (this.inetUtils != null && this.inetUtils.isPresent()) { | ||
| Object utils = this.inetUtils.get(); | ||
| try { | ||
| java.lang.reflect.Method m = utils.getClass().getMethod("findFirstNonLoopbackHostInfo"); |
There was a problem hiding this comment.
I think this may easily break in case of native compilation.
@erikpetzold your thoughts?
3 participants
This PR implements support for Spring Cloud's InetUtils in DefaultApplicationFactory.getLocalHost().
Closes #1323.