TimothyYe / godns

A dynamic DNS client tool that supports AliDNS, Cloudflare, Google Domains, DNSPod, HE.net & DuckDNS & DreamHost, etc, written in Go.
https://timothyye.github.io/godns/
Apache License 2.0
1.5k stars 220 forks source link

Create generic handler #155

Closed drewgingerich closed 2 years ago

drewgingerich commented 2 years ago

I've noticed that each handler re-implements a lot of common functionality, such as:

I'm proposing we extract this common functionality out of the handlers and place it into a central struct type. The handlers will be left with only the logic needed to work with their associated DNS services. I think this will make the code-base more maintainable and make it easier for contributors to add new handlers.

I also suggest this central struct type be named Handler, and that the existing handlers (now stripped of the common functionality) be renamed to "providers". I think this new naming scheme will disambiguate them from the new Handler struct type and makes sense to me because they're related to the provider config block and each one provides access to a particular DNS service.

I've introduced these changes in a way that makes us able to transition over time instead of all at once. In particular the new Handler struct implements IHandler so it can be used like other handlers. Existing handlers that want to let the new Handler struct take care of all the common logic just need to implement the IDNSProvider interface. Currently only the linode handler is making use of it.

Let me know what you think, and if you have an comments or questions!

TimothyYe commented 2 years ago

Thanks, I think the current handlers are a little bit "heavy", they have to implement a similar "while loop", the only differences are the API calls to the providers. Putting these common functionalities in a generic handler is a good idea.

TimothyYe commented 2 years ago

And I created another branch called refactor, I plan to remove the panic channel from every handler as long as all the handlers are stable.

TimothyYe commented 2 years ago

BTW, I prefer to merge this MR to a new branch generic_handler, it will be merged to the master branch when all the handlers are updated.

drewgingerich commented 2 years ago

BTW, I prefer to merge this MR to a new branch generic_handler, it will be merged to the master branch when all the handlers are updated.

@TimothyYe I can do this, sure! I think you'll need to create the generic_handler branch before I can target it with this PR, though.

TimothyYe commented 2 years ago

@drewgingerich Thanks, I've merged it.