[PATCH v2] Update respond_to? calls for second argument.

[Eric Wong <e@80x24.org>]

Pat Allan <pat@freelancing-gods.com> wrote:
> For reference, here’s the point where Rack became explicit about using the two arguments:
> https://github.com/rack/rack/commit/5f808aa2099841e5daec6cb772a304797879ce6c

Thanks!  It looks like I never noticed that since I always have
Clogger at the outermost layer (to give the most accurate timings),
so BodyProxy would never see it.

> I’m not quite sure where to start with a test, I’m afraid - so
> if you’re able to take care of that, that’d be brilliant.

OK, done (see below)

> As for the C internals - I’m reading what you’ve noted, and
> I’m understanding at a basic level, but you’ve got the deep
> knowledge here, so I’m very happy to go with your decisions :)

No worries, feel free to ping me (+cc ruby-core) if you need
Ruby C API help.

Also, I guess rb_obj_respond_to should be documented as a public
API in Ruby.  Do you think you can open an issue on
https://bugs.ruby-lang.org/ about it that effect?  I admit I'm
a bit clumsy when it comes to browsers, even with w3m :x

Here's the version I'll push out and release:
------8<-------
From: Pat Allan <pat@freelancing-gods.com>
Subject: [PATCH] Update respond_to? calls for second argument.

Rack (since v2) has started explicitly listing the second
(optional) argument for respond_to?, which matches the
underlying Ruby spec. This patch fixes the calls in both C
and Ruby approaches.

[ew: add test, use rb_obj_respond_to if available]
---
 ext/clogger_ext/clogger.c    | 17 +++++++++++++----
 ext/clogger_ext/extconf.rb   |  1 +
 lib/clogger/pure.rb          |  4 ++--
 test/test_clogger_to_path.rb |  9 +++++++++
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 481dd61..fdc23e3 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -963,14 +963,23 @@ static VALUE clogger_init_copy(VALUE clone, VALUE orig)
  * used to delegate +:to_path+ checks for Rack webservers that optimize
  * static file serving
  */
-static VALUE respond_to(VALUE self, VALUE method)
+static VALUE respond_to(int argc, VALUE *argv, VALUE self)
 {
 	struct clogger *c = clogger_get(self);
-	ID id = rb_to_id(method);
+	VALUE method, include_all;
+	ID id;
 
+	rb_scan_args(argc, argv, "11", &method, &include_all);
+	id = rb_to_id(method);
 	if (close_id == id)
 		return Qtrue;
-	return rb_respond_to(c->body, id);
+
+#ifdef HAVE_RB_OBJ_RESPOND_TO
+	return rb_obj_respond_to(c->body, id, RTEST(include_all));
+#endif
+	if (argc == 1)
+		return rb_respond_to(c->body, id);
+	return rb_funcallv(c->body, respond_to_id, argc, argv);
 }
 
 /*
@@ -1044,7 +1053,7 @@ void Init_clogger_ext(void)
 	rb_define_method(cClogger, "wrap_body?", clogger_wrap_body, 0);
 	rb_define_method(cClogger, "reentrant?", clogger_reentrant, 0);
 	rb_define_method(cClogger, "to_path", to_path, 0);
-	rb_define_method(cClogger, "respond_to?", respond_to, 1);
+	rb_define_method(cClogger, "respond_to?", respond_to, -1);
 	rb_define_method(cClogger, "body", body, 0);
 	CONST_GLOBAL_STR(REMOTE_ADDR);
 	CONST_GLOBAL_STR(HTTP_X_FORWARDED_FOR);
diff --git a/ext/clogger_ext/extconf.rb b/ext/clogger_ext/extconf.rb
index 523b314..b2c0891 100644
--- a/ext/clogger_ext/extconf.rb
+++ b/ext/clogger_ext/extconf.rb
@@ -22,6 +22,7 @@ begin
   have_func('rb_thread_call_without_gvl', 'ruby/thread.h')
   have_func('rb_thread_blocking_region', 'ruby.h')
   have_func('rb_thread_io_blocking_region', 'ruby.h')
+  have_func('rb_obj_respond_to', 'ruby/intern.h')
   create_makefile('clogger_ext')
 rescue Object => err
   warn "E: #{err.inspect}"
diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index 77f81b4..fddfe79 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -77,8 +77,8 @@ class Clogger
     @logger.respond_to?(:fileno) ? @logger.fileno : nil
   end
 
-  def respond_to?(m)
-    :close == m.to_sym || @body.respond_to?(m)
+  def respond_to?(method, include_all=false)
+    :close == method.to_sym || @body.respond_to?(method, include_all)
   end
 
   def to_path
diff --git a/test/test_clogger_to_path.rb b/test/test_clogger_to_path.rb
index b437695..f74b991 100644
--- a/test/test_clogger_to_path.rb
+++ b/test/test_clogger_to_path.rb
@@ -14,6 +14,10 @@ class MyBody < Struct.new(:to_path, :closed)
   def close
     self.closed = true
   end
+
+private
+  def privtest
+  end
 end
 
 class TestCloggerToPath < Test::Unit::TestCase
@@ -59,6 +63,11 @@ class TestCloggerToPath < Test::Unit::TestCase
     status, headers, body = app.call(@req)
     assert_instance_of(Clogger, body)
     check_body(body)
+
+    assert ! body.respond_to?(:privtest)
+    assert body.respond_to?(:privtest, true)
+    assert ! body.respond_to?(:privtest, false)
+
     assert logger.string.empty?
     assert_equal tmp.path, body.to_path
     body.close
-- 
EW