Feature #4788: resolv.rb refactoring

とりあえず思いつかなかったのでパッチでは each_resource_ という名前にしてしまいましたが、fetch_resource という名前でどうでしょうか。

• Assignee set to akr (Akira Tanaka)

I have a vested interest in this patch since I am the developer of RubyDNS. This update will allow RubyDNS to hook into resolve.rb more efficiently. Right now, I have to duplicate code in resolve.rb to get the right behaviour. Also, I originally suggested then name fetch_resource and thus I support Makoto Kishimoto's proposal about this change. Lets work to get this updated so that I can add good support for RubyDNS and Ruby 1.9. Thanks.

• Target version changed from 1.9.3 to 2.0.0

• File resolv_rb_patch.txt added

2011/5/27 Makoto Kishimoto <[e-mail]>: > > このようなモンキーパッチが（私のコードではありませんが） > https://github.com/ioquatix/rubydns/blob/master/lib/rubydns/resolv.rb > #3835 （ [ruby-core:32407] ）の結果、動かなくなっていたのでパッチを検討していたわけですが、 > 結論としてresolv.rbに以下のようなリファクタリングを施すのがいいのではないかと考えました。 なぜそうするのがいいのか書いていないのでなんとも言い難いものがあります。 モンキーパッチがうまくいかないから、という理由は受け入れ難いです。 後で調べるはめになったときのために、 変更するときには意図をどこかに残しておきたいと思うのですが、 上記はその記述として十分なものとは思えません。 -- [田中 哲][たなか あきら][Tanaka Akira]

元の問題は、lib/resolv.rb が、TCP へのフォールバックに対応して、その際に make_requester --> make_udp_requester という名前の変更があったために、 （公開インターフェースではない）それに依存していたコードが動かなくなった、 というものです。 しかし、make_requester のような内部の非常に低いレイヤにあるメソッドに rubydns ライブラリのコードが依存していた理由は、DNS#each_resource メソッド中に、個々のリソースを取得するコードが一体不可分に含まれてしまって いるため、DNS プロキシのようなものを作る場合、each_resource メソッド中の リソース取得部分のコードの duplicate が、このようなライブラリの実装において 不可避であったためです。 パッチで示したように、リソース取得部分を切り離し、また、一種のコールバックで ある extract_resources を、現状のハードコーディング状態から抽象化する リファクタリングによって、ライブラリ側でのコード duplicate が必要なくなります。 パッチで追加される fetch_resource メソッドのユースケースとして、 ライブラリ側のコードがどうなるかというパッチを示します。 --- resolv.rb.orig 2011-05-27 18:15:59.000000000 +0900 +++ resolv.rb 2011-08-03 10:38:43.000000000 +0900 @@ -24,6 +24,13 @@ # This allows such responses to be passed upstream with little or no # modification/reinterpretation. def query(name, typeclass) + if respond_to?(:fetch_resource) then + fetch_resource(name, typeclass) do |reply, reply_name| + return reply, reply_name + end + return + end + lazy_initialize requester = make_requester senders = {}

It would be nice to get some movement on this - I'm getting bug reports from people trying to use 1.9.x and RubyDNS. Kind regards, Samuel

• Status changed from Open to Assigned

Hi, still having users with problems and no consistent way to solve it. Merging this patch would be a great addition. Let me know if you require any further support or have any questions.

Here is a translation into English for the most recent message from Makoto: The original problem is that 'lib/resolv.rb' had a fallback to TCP which was broken. There was a patch (#3835) which renamed ‘make_requester' to 'make_udp_requester’. This is not a public interface so the code which depended on it stopped working. However, the reason why RubyDNS depended on such a method is because the high level interface performs breaks the response up into individual records but we are actually interested in the response in its entirety. This is specifically a problem when creating a DNS proxy where you want to forward requests with minimal changes. Duplicating the code for each_resource was unavoidable in the implementation of RubyDNS. As in the patch provided by Makoto, code duplication can be reduced by removing the direct connection between each_resource and fetch_resource, and providing a block to be executed per successful response. A second patch shows how RubyDNS can be simplified once the proposed change is applied.