Add getRequiredAddress() and getMaskBits() to IpAddressMatcher.java

[wtigerhyunsu]

Closes gh-16693

When reviewing the issue, the original request was to add a toString() method to IpAddressMatcher.
However, what the reporter actually needed was the hostAddress that had been set in the IpAddressMatcher.

I didn’t think it was appropriate to override toString() just to return the hostAddress, because toString() should ideally reflect the entire object state or be used for debugging purposes—not to expose a specific internal value. So I decided not to implement it that way.

As for getRequiredAddress(), I believed that it was sufficient to return the raw InetAddress directly without wrapping or transforming it to a hostAddress. This way, users can still access what they need without inconvenience.

On the other hand, since getMaskBits() is tightly coupled with the subnet concept and is inherently derived from the original hostAddress, I felt it was appropriate to expose that detail explicitly through this method.

[jzheaux]

Based on the OP's description, it is only needed for logging. It's quite common for a toString to output the entire object's state, less any sensitive information.

Also, adding a toString implementation will bring this request matcher into alignment with #5446. It would probably be good to consult the toString implementations for other request matchers to simplify interpreting logs.

Are you able to change the PR to implement toString instead of adding getter methods?

[jzheaux]

To be clear, I understand that getters were suggested and the OP agreed that they may have value. Unless there is a present need, we usually wait to add to the public API.

[wtigerhyunsu]

Thanks @jzheaux !

In my experience with Spring, toString() is often used to output the full object structure, like:

IpAddressMatcher(requiredAddress=IpAddressMatcher(hostAddress=test, hostName=test), nMaskBits=1)
However, based on the original request from the OP, it seems they were expecting something simpler, like:

192.102.101.1/24
That’s why I was considering adding a method like getIpAddressString() instead of overriding toString() — to return only the IP/subnet in a concise format.

I’d really appreciate your thoughts on which direction would be more appropriate.

Also, I’m curious — from your experience, which approach is more commonly used for logging purposes in the Spring ecosystem:
overriding toString() with a simplified output, or providing a dedicated method like getIpAddressString()?

[jzheaux]

Good questions, @wtigerhyunsu.

The primary context of my recommendation is about noticing the overlap between the original request and the fact that adding IpAddressMatcher#toString would bring it into alignment with the rest of the request matchers. I'm open to what the implementation might look like.

I prefer printing out the IP Address in a standardized format (CIDR in this case) in the same way that I prefer printing out the method and path as GET /my/path in PathPatternRequestMatcher instead of method=GET, pathPattern=/my/path. I believe that makes the log easier and more universally readable. In this case, I'd imagine the toString to print out:

IpAddress [192.168.1.0/24]

which follows the convention for RequestMatcher implementations. I like this because when a composite request matcher is logged, then it makes it easier to reason about:

And [PathPattern [GET /my/path], OR [IpAddress [192.168.1.0/24], IpAddress [127.0.0.1/32]]

or providing a dedicated method like getIpAddressString()?

We can always add another public method should the need arise. I'd like to wait on getIpAddressString until folks demonstrate a need for it. I agree that if folks need to get the CIDR string, it would be a separate method. It's important to remember that the app had to give the CIDR representation to the constructor, so many times the app already has it. Wherever they got it from originally is likely a better source for that information anyway.