Aggregations Refactor: Refactor Range Aggregations #15399
Conversation
| public static String createCIDR(long ipAddress, int networkMask) { | ||
| long blockSize = 1L << (32 - networkMask); | ||
| return octetsToCIDR(longToOctets(ipAddress - (ipAddress & (blockSize - 1))), networkMask); | ||
| } |
There was a problem hiding this comment.
I don't have a good alternative to propose but wish we did not have to add this public method to Cidrs solely for testing purposes
There was a problem hiding this comment.
if we keep it here, maybe we should at least require an address that is valid for the given mask like cidrMaskToMinMax expects?
There was a problem hiding this comment.
So the reason for this method was so I could take a random IP address and a random networkMask and turn them into an appropriate CIDR. I'm not sure how to do that without either having this method or recreating a lot of the logic in this class in the test?
There was a problem hiding this comment.
agreed that's what I said I don't have a good alternative so let's keep it. But can we make it less lenient and expect an ip address that is compatible with the mask and move the - (ipAddress & (blockSize - 1)) part to the ipv4 range tests?
|
I left two minor comments but it looks good to me otherwise. |
clintongormley
added
:Search/Search
No description provided.