Skip to content
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

Implement pluggable I/O? #253

Closed
ivanr opened this issue May 16, 2022 · 8 comments
Closed

Implement pluggable I/O? #253

ivanr opened this issue May 16, 2022 · 8 comments

Comments

@ivanr
Copy link

ivanr commented May 16, 2022

As a workaround for #223 we maintain a limited fork of some dnsjava classes. We wanted to use our own NioTcpClient and NioUdpClient classes, but SimpleResolver calls their static methods so we had to fork that too. This broke down with dnsjava 3.5.1, which now calls Message#getGeneratedTSIG(), which is package-private.

For use cases like ours I think a cleaner approach would be to make the I/O pluggable so that we can use dnsjava directly while supplying our own I/O classes. If this is something you'd accept, is there an approach that you would like to take? Or should I propose something?

@ibauersachs
Copy link
Member

If you have an idea how this could look like, then a proposal (and possibly the implementation afterwards) would be welcome. Ideally, the changes wouldn't impact existing users, i.e. the current I/O should be the default.

@ivanr
Copy link
Author

ivanr commented May 16, 2022

Would you be happy with an addition of a factory class that by default returns instances of the existing TCP and UDP client classes, but can be configured to return something else? Static calls to TCP and UDP clients [in the dnsjava code] would be replaced with static calls to this class instead.

@ibauersachs
Copy link
Member

That's seems okay. I wonder if using Java's ServiceLoader.load would make sense, but it's probably overkill.

@ivanr
Copy link
Author

ivanr commented May 30, 2022

I agree. I don't think the increased complexity of ServiceLoader brings anything. With the factory approach I may also try to support per-instance customisation. I will try to schedule some time for this soon.

@ivanr
Copy link
Author

ivanr commented Aug 15, 2022

@ibauersachs Just to let you know that I haven't dropped this. It's just that I've been extraordinarily busy. I will get to this eventually.

@chrisruffalo
Copy link
Contributor

I ran into this issue so I took a rough swing at it: #302

Obviously very rough. The main issue I ran into is the repeated use of package private on methods that may, in fact, form part of a more public API.

@ibauersachs
Copy link
Member

I tried to keep a lot of things package-private to avoid a broad API surface that cannot be changed later. Are there any particular methods you would like to see public?

@chrisruffalo
Copy link
Contributor

chrisruffalo commented Oct 30, 2023

I think that mostly it was frustration in not being able to overwrite SimpleResolver but instead the need to copy it in it's entirety and make it part of the org.xbill.DNS package which wouldn't work out cross-jar. I think making the resolver client factory (probably a bad name for it) helps ameliorate this issue a lot. I don't remember any methods in particular and I'm only part of the way into doing what I want. (More control over the internals of SimpleResolver while re-using the send/receive logic. I also might want to use the vertx client to send the data... still investigating.)

@ibauersachs ibauersachs added this to the v3.6 milestone Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants