Skip to content

Add InetAddressAdapter for serialization of all InetAddress variations #288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

Mechite
Copy link
Contributor

@Mechite Mechite commented Sep 16, 2024

Perhaps adding a usage for valid IPv4 and IPv6 serialization/de-serialization, in blackbox-test, would be desired.

Perhaps adding a usage for valid IPv4 and IPv6 serialization/de-serialization, in blackbox-test, would be desired.

Signed-off-by: Mechite <[email protected]>
@rbygrave
Copy link
Contributor

valid IPv4 and IPv6 serialization/de-serialization, in blackbox-test

Yes please.

@Mechite
Copy link
Contributor Author

Mechite commented Sep 16, 2024

This has been done, and by extension I implemented support for Inet4Address and Inet6Address.

I have seen beans be created with these directly used, and they are part of public API.

However, it appears not ideal practice to use them unless one depends on their unique methods, rather than the abstraction, but supporting them is trivial, and I did see it used many times, maybe not even for the unique methods, but rather to just specify in the model of a bean whether the expected IP is IPv4 or IPv6.

The JDK network API is designed to be generic and support both IPv4 and IPv6 without favoring one over the other, but this has proven over time to not be good practice.
Explicit control is generally desired, hence the use of these, and hence them remaining public API and not just an implementation detail

@SentryMan SentryMan merged commit 92e0461 into avaje:main Sep 16, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants