From e4f738709482d95e49552f7ddfda800e1b4a6baf Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 14 Jan 2011 13:59:15 -0800 Subject: remove Clogger::ToPath proxy class We can just make Clogger#respond_to? smarter and forward everything except :close to the body we're proxying. --- ext/clogger_ext/clogger.c | 36 +++++++++++++++--------------------- lib/clogger.rb | 9 --------- lib/clogger/pure.rb | 27 +++++++++++++-------------- test/test_clogger_to_path.rb | 23 +++++++++++++++++------ 4 files changed, 45 insertions(+), 50 deletions(-) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index d96b046..cc66f3e 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -124,8 +124,8 @@ static ID sq_brace_id; static ID new_id; static ID to_path_id; static ID to_io_id; +static ID respond_to_id; static VALUE cClogger; -static VALUE cToPath; static VALUE mFormat; static VALUE cHeaderHash; @@ -807,9 +807,6 @@ static VALUE clogger_call(VALUE self, VALUE env) rv = ccall(c, env); assert(!OBJ_FROZEN(rv) && "frozen response array"); - - if (rb_respond_to(c->body, to_path_id)) - self = rb_funcall(cToPath, new_id, 1, self); rb_ary_store(rv, 2, self); return rv; @@ -840,25 +837,24 @@ static VALUE clogger_init_copy(VALUE clone, VALUE orig) #define CONST_GLOBAL_STR(val) CONST_GLOBAL_STR2(val, #val) -#ifdef RSTRUCT_PTR -# define ToPath_clogger(tp) RSTRUCT_PTR(tp)[0] -#else -static ID clogger_id; -# define ToPath_clogger(tp) rb_funcall(tp,clogger_id,0) -#endif +static VALUE respond_to(VALUE self, VALUE method) +{ + struct clogger *c = clogger_get(self); + ID id = rb_to_id(method); + + if (close_id == id) + return Qtrue; + return rb_respond_to(c->body, id); +} static VALUE to_path(VALUE self) { - VALUE my_clogger = ToPath_clogger(self); - struct clogger *c = clogger_get(my_clogger); + struct clogger *c = clogger_get(self); VALUE path = rb_funcall(c->body, to_path_id, 0); struct stat sb; int rv; unsigned devfd; - const char *cpath; - - Check_Type(path, T_STRING); - cpath = RSTRING_PTR(path); + const char *cpath = StringValuePtr(path); /* try to avoid an extra path lookup */ if (rb_respond_to(c->body, to_io_id)) @@ -898,6 +894,7 @@ void Init_clogger_ext(void) new_id = rb_intern("new"); to_path_id = rb_intern("to_path"); to_io_id = rb_intern("to_io"); + respond_to_id = rb_intern("respond_to?"); cClogger = rb_define_class("Clogger", rb_cObject); mFormat = rb_define_module_under(cClogger, "Format"); rb_define_alloc_func(cClogger, clogger_alloc); @@ -909,6 +906,8 @@ void Init_clogger_ext(void) rb_define_method(cClogger, "fileno", clogger_fileno, 0); 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); CONST_GLOBAL_STR(REMOTE_ADDR); CONST_GLOBAL_STR(HTTP_X_FORWARDED_FOR); CONST_GLOBAL_STR(REQUEST_METHOD); @@ -927,9 +926,4 @@ void Init_clogger_ext(void) tmp = rb_const_get(rb_cObject, rb_intern("Rack")); tmp = rb_const_get(tmp, rb_intern("Utils")); cHeaderHash = rb_const_get(tmp, rb_intern("HeaderHash")); - cToPath = rb_const_get(cClogger, rb_intern("ToPath")); - rb_define_method(cToPath, "to_path", to_path, 0); -#ifndef RSTRUCT_PTR - clogger_id = rb_intern("clogger"); -#endif } diff --git a/lib/clogger.rb b/lib/clogger.rb index d0f6fb4..a64ca09 100644 --- a/lib/clogger.rb +++ b/lib/clogger.rb @@ -40,15 +40,6 @@ class Clogger :request_uri => 7 } - # proxy class to avoid clobbering the +to_path+ optimization when - # using static files - class ToPath < Struct.new(:clogger) - def each(&block); clogger.each(&block); end - def close; clogger.close; end - - # to_path is defined in Clogger::Pure or the C extension - end - private CGI_ENV = Regexp.new('\A\$(' << diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb index 4146cce..6613686 100644 --- a/lib/clogger/pure.rb +++ b/lib/clogger/pure.rb @@ -43,7 +43,6 @@ class Clogger wbody.status = status wbody.headers = headers wbody.body = body - wbody = Clogger::ToPath.new(wbody) if body.respond_to?(:to_path) return [ status, headers, wbody ] end log(env, status, headers) @@ -77,6 +76,19 @@ class Clogger @logger.respond_to?(:fileno) ? @logger.fileno : nil end + def respond_to?(m) + :close == m.to_sym || @body.respond_to?(m) + end + + def to_path + rv = @body.to_path + # try to avoid unnecessary path lookups with to_io.stat instead of + # File.size + @body_bytes_sent = + @body.respond_to?(:to_io) ? @body.to_io.stat.size : File.size(rv) + rv + end + private def byte_xs(s) @@ -151,17 +163,4 @@ private end }.join('') end - - class ToPath - def to_path - rv = (body = clogger.body).to_path - - # try to avoid unnecessary path lookups with to_io.stat instead of - # File.stat - clogger.body_bytes_sent = - (body.respond_to?(:to_io) ? body.to_io.stat : File.stat(rv)).size - rv - end - end - end diff --git a/test/test_clogger_to_path.rb b/test/test_clogger_to_path.rb index b25ec46..00767dc 100644 --- a/test/test_clogger_to_path.rb +++ b/test/test_clogger_to_path.rb @@ -31,6 +31,14 @@ class TestCloggerToPath < Test::Unit::TestCase } end + def check_body(body) + assert body.respond_to?(:to_path) + assert body.respond_to?("to_path") + + assert ! body.respond_to?(:to_Path) + assert ! body.respond_to?("to_Path") + end + def test_wraps_to_path logger = StringIO.new tmp = Tempfile.new('') @@ -49,8 +57,8 @@ class TestCloggerToPath < Test::Unit::TestCase end.to_app status, headers, body = app.call(@req) - assert_instance_of(Clogger::ToPath, body) - assert body.respond_to?(:to_path) + assert_instance_of(Clogger, body) + check_body(body) assert logger.string.empty? assert_equal tmp.path, body.to_path body.close @@ -76,8 +84,8 @@ class TestCloggerToPath < Test::Unit::TestCase end.to_app status, headers, body = app.call(@req) - assert_instance_of(Clogger::ToPath, body) - assert body.respond_to?(:to_path) + assert_instance_of(Clogger, body) + check_body(body) assert logger.string.empty? assert_equal "/dev/fd/#{tmp.fileno}", body.to_path body.close @@ -109,8 +117,9 @@ class TestCloggerToPath < Test::Unit::TestCase end.to_app status, headers, body = app.call(@req) - assert_instance_of(Clogger::ToPath, body) - assert body.respond_to?(:to_path) + assert_instance_of(Clogger, body) + check_body(body) + body.to_path assert_kind_of IO, tmp.instance_variable_get(:@to_io_called) assert logger.string.empty? @@ -135,6 +144,8 @@ class TestCloggerToPath < Test::Unit::TestCase end.to_app status, headers, body = app.call(@req) assert_instance_of(Clogger, body) + assert ! body.respond_to?(:to_path) + assert ! body.respond_to?("to_path") assert logger.string.empty? body.close assert ! logger.string.empty? -- cgit v1.2.3-24-ge0c7