From 335a31524055c7dd79618ea79b3c18d827e25d3d Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 10 Nov 2008 14:16:43 -0600 Subject: Add simple case when DB calculations returns 0 instead of 0.0 [#1346 state:resolved] --- activerecord/lib/active_record/calculations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index dd90580b3d..6f4e02b430 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -286,7 +286,7 @@ module ActiveRecord case operation when 'count' then value.to_i when 'sum' then type_cast_using_column(value || '0', column) - when 'avg' then value && value.to_f.to_d + when 'avg' then value && (value == 0 ? 0.0.to_d : value.to_d) else type_cast_using_column(value, column) end end -- cgit v1.2.3 From 5db9f9b3ad47fadf0b3f12ada1c2ea7b9c15ded5 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 10 Nov 2008 17:41:07 -0800 Subject: Pare down object creation during route building --- .../lib/action_controller/routing/builder.rb | 53 ++++++++++------------ .../lib/action_controller/routing/routing_ext.rb | 4 ++ 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/actionpack/lib/action_controller/routing/builder.rb b/actionpack/lib/action_controller/routing/builder.rb index 7b888fa8d2..d4e501e780 100644 --- a/actionpack/lib/action_controller/routing/builder.rb +++ b/actionpack/lib/action_controller/routing/builder.rb @@ -1,23 +1,16 @@ module ActionController module Routing class RouteBuilder #:nodoc: - attr_accessor :separators, :optional_separators + attr_reader :separators, :optional_separators + attr_reader :separator_regexp, :nonseparator_regexp, :interval_regexp def initialize - self.separators = Routing::SEPARATORS - self.optional_separators = %w( / ) - end - - def separator_pattern(inverted = false) - "[#{'^' if inverted}#{Regexp.escape(separators.join)}]" - end - - def interval_regexp - Regexp.new "(.*?)(#{separators.source}|$)" - end + @separators = Routing::SEPARATORS + @optional_separators = %w( / ) - def multiline_regexp?(expression) - expression.options & Regexp::MULTILINE == Regexp::MULTILINE + @separator_regexp = /[#{Regexp.escape(separators.join)}]/ + @nonseparator_regexp = /\A([^#{Regexp.escape(separators.join)}]+)/ + @interval_regexp = /(.*?)(#{separator_regexp}|$)/ end # Accepts a "route path" (a string defining a route), and returns the array @@ -30,7 +23,7 @@ module ActionController rest, segments = path, [] until rest.empty? - segment, rest = segment_for rest + segment, rest = segment_for(rest) segments << segment end segments @@ -39,20 +32,20 @@ module ActionController # A factory method that returns a new segment instance appropriate for the # format of the given string. def segment_for(string) - segment = case string - when /\A:(\w+)/ - key = $1.to_sym - case key - when :controller then ControllerSegment.new(key) - else DynamicSegment.new key - end - when /\A\*(\w+)/ then PathSegment.new($1.to_sym, :optional => true) - when /\A\?(.*?)\?/ - StaticSegment.new($1, :optional => true) - when /\A(#{separator_pattern(:inverted)}+)/ then StaticSegment.new($1) - when Regexp.new(separator_pattern) then - DividerSegment.new($&, :optional => (optional_separators.include? $&)) - end + segment = + case string + when /\A:(\w+)/ + key = $1.to_sym + key == :controller ? ControllerSegment.new(key) : DynamicSegment.new(key) + when /\A\*(\w+)/ + PathSegment.new($1.to_sym, :optional => true) + when /\A\?(.*?)\?/ + StaticSegment.new($1, :optional => true) + when nonseparator_regexp + StaticSegment.new($1) + when separator_regexp + DividerSegment.new($&, :optional => optional_separators.include?($&)) + end [segment, $~.post_match] end @@ -98,7 +91,7 @@ module ActionController if requirement.source =~ %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" end - if multiline_regexp?(requirement) + if requirement.multiline? raise ArgumentError, "Regexp multiline option not allowed in routing requirements: #{requirement.inspect}" end segment.regexp = requirement diff --git a/actionpack/lib/action_controller/routing/routing_ext.rb b/actionpack/lib/action_controller/routing/routing_ext.rb index 5f4ba90d0c..4a82b2af5f 100644 --- a/actionpack/lib/action_controller/routing/routing_ext.rb +++ b/actionpack/lib/action_controller/routing/routing_ext.rb @@ -27,6 +27,10 @@ class Regexp #:nodoc: Regexp.new("|#{source}").match('').captures.length end + def multiline? + options & MULTILINE == MULTILINE + end + class << self def optionalize(pattern) case unoptionalize(pattern) -- cgit v1.2.3 From 278b6cd9529f33286449a9be18f1903687814d3f Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 10 Nov 2008 19:53:53 -0800 Subject: Eliminate excess Regexp creation due to capture counting --- actionpack/lib/action_controller/routing/route.rb | 2 +- .../lib/action_controller/routing/segments.rb | 28 +++++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_controller/routing/route.rb b/actionpack/lib/action_controller/routing/route.rb index 3b2cb28545..a610ec7e84 100644 --- a/actionpack/lib/action_controller/routing/route.rb +++ b/actionpack/lib/action_controller/routing/route.rb @@ -219,7 +219,7 @@ module ActionController next_capture = 1 extraction = segments.collect do |segment| x = segment.match_extraction(next_capture) - next_capture += Regexp.new(segment.regexp_chunk).number_of_captures + next_capture += segment.number_of_captures x end extraction.compact diff --git a/actionpack/lib/action_controller/routing/segments.rb b/actionpack/lib/action_controller/routing/segments.rb index e5f174ae2c..f6b03edcca 100644 --- a/actionpack/lib/action_controller/routing/segments.rb +++ b/actionpack/lib/action_controller/routing/segments.rb @@ -13,6 +13,10 @@ module ActionController @is_optional = false end + def number_of_captures + Regexp.new(regexp_chunk).number_of_captures + end + def extraction_code nil end @@ -84,6 +88,10 @@ module ActionController optional? ? Regexp.optionalize(chunk) : chunk end + def number_of_captures + 0 + end + def build_pattern(pattern) escaped = Regexp.escape(value) if optional? && ! pattern.empty? @@ -194,10 +202,16 @@ module ActionController end end + def number_of_captures + if regexp + regexp.number_of_captures + 1 + else + 1 + end + end + def build_pattern(pattern) - chunk = regexp_chunk - chunk = "(#{chunk})" if Regexp.new(chunk).number_of_captures == 0 - pattern = "#{chunk}#{pattern}" + pattern = "#{regexp_chunk}#{pattern}" optional? ? Regexp.optionalize(pattern) : pattern end @@ -230,6 +244,10 @@ module ActionController "(?i-:(#{(regexp || Regexp.union(*possible_names)).source}))" end + def number_of_captures + 1 + end + # Don't URI.escape the controller name since it may contain slashes. def interpolation_chunk(value_code = local_name) "\#{#{value_code}.to_s}" @@ -275,6 +293,10 @@ module ActionController regexp || "(.*)" end + def number_of_captures + regexp ? regexp.number_of_captures : 1 + end + def optionality_implied? true end -- cgit v1.2.3 From cbb38bbdba0f7cfb628a0f8716e79d0d079fd7bf Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 10 Nov 2008 21:39:05 -0800 Subject: Only track new constant definitions when we're reloading dependencies --- activesupport/lib/active_support/dependencies.rb | 12 ++++++++++-- activesupport/test/dependencies_test.rb | 10 +++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 3d871eec11..fe568d6127 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -138,14 +138,22 @@ module ActiveSupport #:nodoc: end def load_with_new_constant_marking(file, *extras) #:nodoc: - Dependencies.new_constants_in(Object) { load_without_new_constant_marking(file, *extras) } + if Dependencies.load? + Dependencies.new_constants_in(Object) { load_without_new_constant_marking(file, *extras) } + else + load_without_new_constant_marking(file, *extras) + end rescue Exception => exception # errors from loading file exception.blame_file! file raise end def require(file, *extras) #:nodoc: - Dependencies.new_constants_in(Object) { super } + if Dependencies.load? + Dependencies.new_constants_in(Object) { super } + else + super + end rescue Exception => exception # errors from required file exception.blame_file! file raise diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 6c3bd1a4fd..fe04b91f2b 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -694,17 +694,17 @@ class DependenciesTest < Test::Unit::TestCase with_loading 'autoloading_fixtures' do ActiveSupport::Dependencies.mechanism = :require 2.times do - assert_raise(NameError) {"RaisesNameError".constantize} + assert_raise(NameError) { assert_equal 123, ::RaisesNameError::FooBarBaz } end end end def test_autoload_doesnt_shadow_name_error with_loading 'autoloading_fixtures' do - assert !defined?(::RaisesNameError), "::RaisesNameError is defined but it hasn't been referenced yet!" + Object.send(:remove_const, :RaisesNameError) if defined?(::RaisesNameError) 2.times do begin - ::RaisesNameError.object_id + ::RaisesNameError::FooBarBaz.object_id flunk 'should have raised NameError when autoloaded file referenced FooBarBaz' rescue NameError => e assert_equal 'uninitialized constant RaisesNameError::FooBarBaz', e.message @@ -712,9 +712,9 @@ class DependenciesTest < Test::Unit::TestCase assert !defined?(::RaisesNameError), "::RaisesNameError is defined but it should have failed!" end - assert !defined?(RaisesNameError) + assert !defined?(::RaisesNameError) 2.times do - assert_raise(NameError) { RaisesNameError } + assert_raise(NameError) { ::RaisesNameError } assert !defined?(::RaisesNameError), "::RaisesNameError is defined but it should have failed!" end end -- cgit v1.2.3