From c366fee263c578c2363b2242d61b935d8b556094 Mon Sep 17 00:00:00 2001 From: Dan Kubb Date: Mon, 14 Jul 2008 22:49:32 -0700 Subject: [PATCH 01/10] Initial addition of GROUP BY support --- .../dm-aggregates/adapters/data_objects_adapter.rb | 8 +++----- dm-aggregates/lib/dm-aggregates/collection.rb | 12 +++++++++++- dm-aggregates/lib/dm-aggregates/functions.rb | 10 +++++----- dm-aggregates/lib/dm-aggregates/model.rb | 12 +++++++++++- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb b/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb index ef1e926a..c7b8a6aa 100644 --- a/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb +++ b/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb @@ -30,14 +30,12 @@ module SQL def aggregate_read_statement(aggregate_function, property, query) statement = "SELECT #{aggregate_field_statement(query.repository, aggregate_function, property, query.links.any?)}" + statement << ", #{fields_statement(query)}" unless query.fields.empty? statement << " FROM #{quote_table_name(query.model.storage_name(query.repository.name))}" statement << links_statement(query) if query.links.any? statement << " WHERE #{conditions_statement(query)}" if query.conditions.any? - - # TODO: when GROUP BY support added, uncomment this, and (by default) have - # it sort on the non-aggregate fields being SELECTed - #statement << " ORDER BY #{order_statement(query)}" if query.order.any? - + statement << " GROUP BY #{fields_statement(query)}" if query.unique? + statement << " ORDER BY #{order_statement(query)}" if query.order.any? statement << " LIMIT #{quote_column_value(query.limit)}" if query.limit statement << " OFFSET #{quote_column_value(query.offset)}" if query.offset && query.offset > 0 statement diff --git a/dm-aggregates/lib/dm-aggregates/collection.rb b/dm-aggregates/lib/dm-aggregates/collection.rb index 1ffe0db3..753d9997 100644 --- a/dm-aggregates/lib/dm-aggregates/collection.rb +++ b/dm-aggregates/lib/dm-aggregates/collection.rb @@ -5,7 +5,17 @@ class Collection private def with_repository_and_property(*args, &block) - query = args.last.respond_to?(:merge) ? args.pop : {} + query = args.last.respond_to?(:merge) ? args.pop : {} + + if query.kind_of?(Hash) + if query.has_key?(:fields) && query[:fields].any? + query[:unique] = true + query[:order] ||= query[:fields] + else + query[:fields] = [] + end + end + property_name = args.first query = scoped_query(query) diff --git a/dm-aggregates/lib/dm-aggregates/functions.rb b/dm-aggregates/lib/dm-aggregates/functions.rb index 3c8253a5..a85e9771 100644 --- a/dm-aggregates/lib/dm-aggregates/functions.rb +++ b/dm-aggregates/lib/dm-aggregates/functions.rb @@ -21,7 +21,7 @@ module Aggregates # @public def count(*args) with_repository_and_property(*args) do |repository,property,query| - repository.count(property, query.merge(:limit => 1)) + repository.count(property, query) end end @@ -42,7 +42,7 @@ def count(*args) def min(*args) with_repository_and_property(*args) do |repository,property,query| check_property_is_number(property) - repository.min(property, query.merge(:limit => 1)) + repository.min(property, query) end end @@ -63,7 +63,7 @@ def min(*args) def max(*args) with_repository_and_property(*args) do |repository,property,query| check_property_is_number(property) - repository.max(property, query.merge(:limit => 1)) + repository.max(property, query) end end @@ -84,7 +84,7 @@ def max(*args) def avg(*args) with_repository_and_property(*args) do |repository,property,query| check_property_is_number(property) - repository.avg(property, query.merge(:limit => 1)) + repository.avg(property, query) end end @@ -105,7 +105,7 @@ def avg(*args) def sum(*args) with_repository_and_property(*args) do |repository,property,query| check_property_is_number(property) - repository.sum(property, query.merge(:limit => 1)) + repository.sum(property, query) end end diff --git a/dm-aggregates/lib/dm-aggregates/model.rb b/dm-aggregates/lib/dm-aggregates/model.rb index 8c5a0978..8bfec5f4 100644 --- a/dm-aggregates/lib/dm-aggregates/model.rb +++ b/dm-aggregates/lib/dm-aggregates/model.rb @@ -5,7 +5,17 @@ module Model private def with_repository_and_property(*args, &block) - query = args.last.respond_to?(:merge) ? args.pop : {} + query = args.last.respond_to?(:merge) ? args.pop : {} + + if query.kind_of?(Hash) + if query.has_key?(:fields) && query[:fields].any? + query[:unique] = true + query[:order] ||= query[:fields] + else + query[:fields] = [] + end + end + property_name = args.first query = scoped_query(query) From 06a9599d5ed86b70441ce9fc15cc296470b42a93 Mon Sep 17 00:00:00 2001 From: Dan Kubb Date: Tue, 15 Jul 2008 12:37:10 -0700 Subject: [PATCH 02/10] Added Model#aggregate and Collection#aggregate methods * New aggregate method allows mixing aggregate function calls in a single query. All other aggregate methods have been refactored to use it under the hood. If :fields option is specified it will automatically GROUP BY the specified fields. If no :order option specified, it will automatically ORDER BY the :fields. * Updated documentation to match latest YARD standards (although unsure about the @example tags, just following convention in dm-core) --- dm-aggregates/lib/dm-aggregates.rb | 3 +- .../adapters/data_objects_adapter.rb | 73 ++++--- .../lib/dm-aggregates/aggregate_functions.rb | 200 ++++++++++++++++++ dm-aggregates/lib/dm-aggregates/collection.rb | 23 +- dm-aggregates/lib/dm-aggregates/functions.rb | 118 ----------- dm-aggregates/lib/dm-aggregates/model.rb | 23 +- dm-aggregates/lib/dm-aggregates/repository.rb | 20 +- .../lib/dm-aggregates/support/symbol.rb | 21 ++ .../spec/integration/aggregates_spec.rb | 83 ++++++-- 9 files changed, 346 insertions(+), 218 deletions(-) create mode 100644 dm-aggregates/lib/dm-aggregates/aggregate_functions.rb delete mode 100644 dm-aggregates/lib/dm-aggregates/functions.rb create mode 100644 dm-aggregates/lib/dm-aggregates/support/symbol.rb diff --git a/dm-aggregates/lib/dm-aggregates.rb b/dm-aggregates/lib/dm-aggregates.rb index 2882dc82..5f482449 100644 --- a/dm-aggregates/lib/dm-aggregates.rb +++ b/dm-aggregates/lib/dm-aggregates.rb @@ -5,8 +5,9 @@ dir = Pathname(__FILE__).dirname.expand_path / 'dm-aggregates' -require dir / 'functions' +require dir / 'aggregate_functions' require dir / 'model' require dir / 'repository' require dir / 'collection' require dir / 'adapters' / 'data_objects_adapter' +require dir / 'support' / 'symbol' diff --git a/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb b/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb index c7b8a6aa..1e9c1872 100644 --- a/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb +++ b/dm-aggregates/lib/dm-aggregates/adapters/data_objects_adapter.rb @@ -1,51 +1,66 @@ module DataMapper module Adapters class DataObjectsAdapter - def count(property, query) - query(aggregate_read_statement(:count, property, query), *query.bind_values).first + def aggregate(query) + with_reader(read_statement(query), query.bind_values) do |reader| + results = [] + + while(reader.next!) do + row = query.fields.zip(reader.values).map do |field,value| + if field.respond_to?(:operator) + send(field.operator, field.target, value) + else + field.typecast(value) + end + end + + results << (query.fields.size > 1 ? row : row[0]) + end + + results + end end - def min(property, query) - min = query(aggregate_read_statement(:min, property, query), *query.bind_values).first - property.typecast(min) + private + + def count(property, value) + value.to_i + end + + def min(property, value) + property.typecast(value) end - def max(property, query) - max = query(aggregate_read_statement(:max, property, query), *query.bind_values).first - property.typecast(max) + def max(property, value) + property.typecast(value) end - def avg(property, query) - avg = query(aggregate_read_statement(:avg, property, query), *query.bind_values).first - property.type == Integer ? avg.to_f : property.typecast(avg) + def avg(property, value) + property.type == Integer ? value.to_f : property.typecast(value) end - def sum(property, query) - sum = query(aggregate_read_statement(:sum, property, query), *query.bind_values).first - property.typecast(sum) + def sum(property, value) + property.typecast(value) end module SQL private - def aggregate_read_statement(aggregate_function, property, query) - statement = "SELECT #{aggregate_field_statement(query.repository, aggregate_function, property, query.links.any?)}" - statement << ", #{fields_statement(query)}" unless query.fields.empty? - statement << " FROM #{quote_table_name(query.model.storage_name(query.repository.name))}" - statement << links_statement(query) if query.links.any? - statement << " WHERE #{conditions_statement(query)}" if query.conditions.any? - statement << " GROUP BY #{fields_statement(query)}" if query.unique? - statement << " ORDER BY #{order_statement(query)}" if query.order.any? - statement << " LIMIT #{quote_column_value(query.limit)}" if query.limit - statement << " OFFSET #{quote_column_value(query.offset)}" if query.offset && query.offset > 0 - statement - rescue => e - DataMapper.logger.error("QUERY INVALID: #{query.inspect} (#{e})") - raise e + alias original_property_to_column_name property_to_column_name + + def property_to_column_name(repository, property, qualify) + case property + when Query::Operator + aggregate_field_statement(repository, property.operator, property.target, qualify) + when Property + original_property_to_column_name(repository, property, qualify) + else + raise ArgumentError, "+property+ must be a DataMapper::Query::Operator or a DataMapper::Property, but was a #{property.class} (#{property.inspect})" + end end def aggregate_field_statement(repository, aggregate_function, property, qualify) - column_name = if aggregate_function == :count && property.nil? + column_name = if aggregate_function == :count && property == :all '*' else property_to_column_name(repository, property, qualify) diff --git a/dm-aggregates/lib/dm-aggregates/aggregate_functions.rb b/dm-aggregates/lib/dm-aggregates/aggregate_functions.rb new file mode 100644 index 00000000..42de6611 --- /dev/null +++ b/dm-aggregates/lib/dm-aggregates/aggregate_functions.rb @@ -0,0 +1,200 @@ +module DataMapper + module AggregateFunctions + # Count results (given the conditions) + # + # @example the count of all friends + # Friend.count + # + # @example the count of all friends older then 18 + # Friend.count(:age.gt => 18) + # + # @example the count of all your female friends + # Friend.count(:conditions => [ 'gender = ?', 'female' ]) + # + # @example the count of all friends with an address (NULL values are not included) + # Friend.count(:address) + # + # @example the count of all friends with an address that are older then 18 + # Friend.count(:address, :age.gt => 18) + # + # @example the count of all your female friends with an address + # Friend.count(:address, :conditions => [ 'gender = ?', 'female' ]) + # + # @param property [Symbol] of the property you with to count (optional) + # @param opts [Hash, Symbol] the conditions + # + # @return [Integer] return the count given the conditions + # + # @api public + def count(*args) + query = args.last.kind_of?(Hash) ? args.pop : {} + property_name = args.first + + if property_name + assert_kind_of 'property', property_by_name(property_name), Property + end + + aggregate(query.merge(:fields => [ property_name ? property_name.count : :all.count ])) + end + + # Get the lowest value of a property + # + # @example the age of the youngest friend + # Friend.min(:age) + # + # @example the age of the youngest female friend + # Friend.min(:age, :conditions => [ 'gender = ?', 'female' ]) + # + # @param property [Symbol] the property you wish to get the lowest value of + # @param opts [Hash, Symbol] the conditions + # + # @return [Integer] return the lowest value of a property given the conditions + # + # @api public + def min(*args) + query = args.last.kind_of?(Hash) ? args.pop : {} + property_name = args.first + + assert_property_type property_name, Integer, Float, BigDecimal, DateTime, Date, Time + + aggregate(query.merge(:fields => [ property_name.min ])) + end + + # Get the highest value of a property + # + # @example the age of the oldest friend + # Friend.max(:age) + # + # @example the age of the oldest female friend + # Friend.max(:age, :conditions => [ 'gender = ?', 'female' ]) + # + # @param property [Symbol] the property you wish to get the highest value of + # @param opts [Hash, Symbol] the conditions + # + # @return [Integer] return the highest value of a property given the conditions + # + # @api public + def max(*args) + query = args.last.kind_of?(Hash) ? args.pop : {} + property_name = args.first + + assert_property_type property_name, Integer, Float, BigDecimal, DateTime, Date, Time + + aggregate(query.merge(:fields => [ property_name.max ])) + end + + # Get the average value of a property + # + # @example the average age of all friends + # Friend.avg(:age) + # + # @example the average age of all female friends + # Friend.avg(:age, :conditions => [ 'gender = ?', 'female' ]) + # + # @param property [Symbol] the property you wish to get the average value of + # @param opts [Hash, Symbol] the conditions + # + # @return [Integer] return the average value of a property given the conditions + # + # @api public + def avg(*args) + query = args.last.kind_of?(Hash) ? args.pop : {} + property_name = args.first + + assert_property_type property_name, Integer, Float, BigDecimal + + aggregate(query.merge(:fields => [ property_name.avg ])) + end + + # Get the total value of a property + # + # @example the total age of all friends + # Friend.sum(:age) + # + # @example the total age of all female friends + # Friend.max(:age, :conditions => [ 'gender = ?', 'female' ]) + # + # @param property [Symbol] the property you wish to get the total value of + # @param opts [Hash, Symbol] the conditions + # + # @return [Integer] return the total value of a property given the conditions + # + # @api public + def sum(*args) + query = args.last.kind_of?(Hash) ? args.pop : {} + property_name = args.first + + assert_property_type property_name, Integer, Float, BigDecimal + + aggregate(query.merge(:fields => [ property_name.sum ])) + end + + # Perform aggregate queries + # + # @example the count of friends + # Friend.aggregate(:all.count) + # + # @example the minimum age, the maximum age and the total age of friends + # Friend.aggregate(:age.min, :age.max, :age.sum) + # + # @example the average age, grouped by gender + # Friend.aggregate(:age.avg, :fields => [ :gender ]) + # + # @param aggregates [Symbol, ...] operators to aggregate with + # @params query [Hash] the conditions + # + # @return [Array,Numeric,DateTime,Date,Time] the results of the + # aggregate query + # + # @api public + def aggregate(*args) + query = args.last.kind_of?(Hash) ? args.pop : {} + + query[:fields] ||= [] + query[:fields] |= args + query[:fields].map! { |f| normalize_field(f) } + query[:order] ||= query[:fields].select { |p| p.kind_of?(Property) } + + raise ArgumentError, 'query[:fields] must not be empty' if query[:fields].empty? + + query = scoped_query(query) + + if query.fields.any? { |p| p.kind_of?(Property) } + query.repository.aggregate(query.update(:unique => true)) + else + query.repository.aggregate(query).first # only return one row + end + end + + private + + def assert_property_type(name, *types) + if name.nil? + raise ArgumentError, 'property name must not be nil' + end + + type = property_by_name(name).type + + unless types.include?(type) + raise ArgumentError, "#{name} must be #{types * ' or '}, but was #{type}" + end + end + + def normalize_field(field) + assert_kind_of 'field', field, Query::Operator, Symbol, Property + + case field + when Query::Operator + if field.target == :all + field + else + field.class.new(property_by_name(field.target), field.operator) + end + when Symbol + property_by_name(field) + when Property + field + end + end + end +end diff --git a/dm-aggregates/lib/dm-aggregates/collection.rb b/dm-aggregates/lib/dm-aggregates/collection.rb index 753d9997..89dbcb76 100644 --- a/dm-aggregates/lib/dm-aggregates/collection.rb +++ b/dm-aggregates/lib/dm-aggregates/collection.rb @@ -1,28 +1,11 @@ module DataMapper class Collection - include Aggregates + include AggregateFunctions private - def with_repository_and_property(*args, &block) - query = args.last.respond_to?(:merge) ? args.pop : {} - - if query.kind_of?(Hash) - if query.has_key?(:fields) && query[:fields].any? - query[:unique] = true - query[:order] ||= query[:fields] - else - query[:fields] = [] - end - end - - property_name = args.first - - query = scoped_query(query) - repository = query.repository - property = properties[property_name] if property_name - - yield repository, property, query + def property_by_name(property_name) + properties[property_name] end end end diff --git a/dm-aggregates/lib/dm-aggregates/functions.rb b/dm-aggregates/lib/dm-aggregates/functions.rb deleted file mode 100644 index a85e9771..00000000 --- a/dm-aggregates/lib/dm-aggregates/functions.rb +++ /dev/null @@ -1,118 +0,0 @@ -module DataMapper - module Aggregates - - # Count results (given the conditions) - # - # ==== Example - # Friend.count # returns count of all friends - # Friend.count(:age.gt => 18) # returns count of all friends older then 18 - # Friend.count(:conditions => [ 'gender = ?', 'female' ]) # returns count of all your female friends - # Friend.count(:address) # returns count of all friends with an address (NULL values are not included) - # Friend.count(:address, :age.gt => 18) # returns count of all friends with an address that are older then 18 - # Friend.count(:address, :conditions => [ 'gender = ?', 'female' ]) # returns count of all your female friends with an address - # - # ==== Parameters - # property:: of the property you with to count (optional) - # opts:: of the conditions - # - # ==== Returns - # :: with the count of the results - #--- - # @public - def count(*args) - with_repository_and_property(*args) do |repository,property,query| - repository.count(property, query) - end - end - - # Get the lowest value of a property - # - # ==== Example - # Friend.min(:age) # returns the age of the youngest friend - # Friend.min(:age, :conditions => [ 'gender = ?', 'female' ]) # returns the age of the youngest female friends - # - # ==== Parameters - # property:: the property you wish to get the lowest value of - # opts:: the conditions - # - # ==== Returns - # :: return the lowest value of a property given the conditions - #--- - # @public - def min(*args) - with_repository_and_property(*args) do |repository,property,query| - check_property_is_number(property) - repository.min(property, query) - end - end - - # Get the highest value of a property - # - # ==== Example - # Friend.max(:age) # returns the age of the oldest friend - # Friend.max(:age, :conditions => [ 'gender = ?', 'female' ]) # returns the age of the oldest female friends - # - # ==== Parameters - # property:: the property you wish to get the highest value of - # opts:: the conditions - # - # ==== Returns - # :: return the highest value of a property given the conditions - #--- - # @public - def max(*args) - with_repository_and_property(*args) do |repository,property,query| - check_property_is_number(property) - repository.max(property, query) - end - end - - # Get the average value of a property - # - # ==== Example - # Friend.avg(:age) # returns the average age of friends - # Friend.avg(:age, :conditions => [ 'gender = ?', 'female' ]) # returns the average age of the female friends - # - # ==== Parameters - # property:: the property you wish to get the average value of - # opts:: the conditions - # - # ==== Returns - # :: return the average value of a property given the conditions - #--- - # @public - def avg(*args) - with_repository_and_property(*args) do |repository,property,query| - check_property_is_number(property) - repository.avg(property, query) - end - end - - # Get the total value of a property - # - # ==== Example - # Friend.sum(:age) # returns total age of all friends - # Friend.max(:age, :conditions => [ 'gender = ?', 'female' ]) # returns the total age of all female friends - # - # ==== Parameters - # property:: the property you wish to get the total value of - # opts:: the conditions - # - # ==== Returns - # :: return the total value of a property given the conditions - #--- - # @public - def sum(*args) - with_repository_and_property(*args) do |repository,property,query| - check_property_is_number(property) - repository.sum(property, query) - end - end - - private - - def check_property_is_number(property) - raise ArgumentError, "+property+ should be an Integer, Float or BigDecimal, but was #{property.nil? ? 'nil' : property.type.class}" unless property && [ Integer, Float, BigDecimal ].include?(property.type) - end - end -end diff --git a/dm-aggregates/lib/dm-aggregates/model.rb b/dm-aggregates/lib/dm-aggregates/model.rb index 8bfec5f4..b234da2a 100644 --- a/dm-aggregates/lib/dm-aggregates/model.rb +++ b/dm-aggregates/lib/dm-aggregates/model.rb @@ -1,28 +1,11 @@ module DataMapper module Model - include Aggregates + include AggregateFunctions private - def with_repository_and_property(*args, &block) - query = args.last.respond_to?(:merge) ? args.pop : {} - - if query.kind_of?(Hash) - if query.has_key?(:fields) && query[:fields].any? - query[:unique] = true - query[:order] ||= query[:fields] - else - query[:fields] = [] - end - end - - property_name = args.first - - query = scoped_query(query) - repository = query.repository - property = properties(repository.name)[property_name] if property_name - - yield repository, property, query + def property_by_name(property_name) + properties(repository.name)[property_name] end end end diff --git a/dm-aggregates/lib/dm-aggregates/repository.rb b/dm-aggregates/lib/dm-aggregates/repository.rb index 2b213220..97eeeb19 100644 --- a/dm-aggregates/lib/dm-aggregates/repository.rb +++ b/dm-aggregates/lib/dm-aggregates/repository.rb @@ -1,23 +1,7 @@ module DataMapper class Repository - def count(property, query) - adapter.count(property, query) - end - - def min(property, query) - adapter.min(property, query) - end - - def max(property, query) - adapter.max(property, query) - end - - def avg(property, query) - adapter.avg(property, query) - end - - def sum(property, query) - adapter.sum(property, query) + def aggregate(query) + adapter.aggregate(query) end end end diff --git a/dm-aggregates/lib/dm-aggregates/support/symbol.rb b/dm-aggregates/lib/dm-aggregates/support/symbol.rb new file mode 100644 index 00000000..348b47a2 --- /dev/null +++ b/dm-aggregates/lib/dm-aggregates/support/symbol.rb @@ -0,0 +1,21 @@ +class Symbol + def count + DataMapper::Query::Operator.new(self, :count) + end + + def min + DataMapper::Query::Operator.new(self, :min) + end + + def max + DataMapper::Query::Operator.new(self, :max) + end + + def avg + DataMapper::Query::Operator.new(self, :avg) + end + + def sum + DataMapper::Query::Operator.new(self, :sum) + end +end # class Symbol diff --git a/dm-aggregates/spec/integration/aggregates_spec.rb b/dm-aggregates/spec/integration/aggregates_spec.rb index d90486fd..e0af5ef2 100644 --- a/dm-aggregates/spec/integration/aggregates_spec.rb +++ b/dm-aggregates/spec/integration/aggregates_spec.rb @@ -7,33 +7,40 @@ # A simplistic example, using with an Integer property class Dragon include DataMapper::Resource - property :id, Serial - property :name, String - property :is_fire_breathing, TrueClass - property :toes_on_claw, Integer - auto_migrate!(:default) + property :id, Serial + property :name, String + property :is_fire_breathing, TrueClass + property :toes_on_claw, Integer + property :birth_at, DateTime + property :birth_on, Date + property :birth_time, Time end - Dragon.create(:name => 'George', :is_fire_breathing => false, :toes_on_claw => 3) - Dragon.create(:name => 'Puff', :is_fire_breathing => true, :toes_on_claw => 4) - Dragon.create(:name => nil, :is_fire_breathing => true, :toes_on_claw => 5) # A more complex example, with BigDecimal and Float properties # Statistics taken from CIA World Factbook: # https://www.cia.gov/library/publications/the-world-factbook/ class Country include DataMapper::Resource - property :id, Integer, :serial => true - property :name, String, :nullable => false + property :id, Serial + property :name, String, :nullable => false property :population, Integer property :birth_rate, Float, :precision => 4, :scale => 2 property :gold_reserve_tonnes, Float, :precision => 6, :scale => 2 property :gold_reserve_value, BigDecimal, :precision => 15, :scale => 1 # approx. value in USD - - auto_migrate!(:default) end + [ Dragon, Country ].each { |m| m.auto_migrate! } + + @birth_at = DateTime.now + @birth_on = Date.parse(@birth_at.to_s) + @birth_time = Time.parse(@birth_at.to_s) + + Dragon.create(:name => 'George', :is_fire_breathing => false, :toes_on_claw => 3, :birth_at => @birth_at, :birth_on => @birth_on, :birth_time => @birth_time) + Dragon.create(:name => 'Puff', :is_fire_breathing => true, :toes_on_claw => 4, :birth_at => @birth_at, :birth_on => @birth_on, :birth_time => @birth_time) + Dragon.create(:name => nil, :is_fire_breathing => true, :toes_on_claw => 5, :birth_at => nil, :birth_on => nil, :birth_time => nil) + gold_kilo_price = 277738.70 @gold_tonne_price = gold_kilo_price * 10000 @@ -144,6 +151,21 @@ def target(klass, target_type) target(Country, target_type).min(:gold_reserve_value).should == BigDecimal('1217050983400.0') end + it 'should provide the lowest value of a DateTime property' do + target(Dragon, target_type).min(:birth_at).should be_kind_of(DateTime) + target(Dragon, target_type).min(:birth_at).to_s.should == @birth_at.to_s + end + + it 'should provide the lowest value of a Date property' do + target(Dragon, target_type).min(:birth_on).should be_kind_of(Date) + target(Dragon, target_type).min(:birth_on).to_s.should == @birth_on.to_s + end + + it 'should provide the lowest value of a Time property' do + target(Dragon, target_type).min(:birth_time).should be_kind_of(Time) + target(Dragon, target_type).min(:birth_time).to_s.should == @birth_time.to_s + end + it 'should provide the lowest value when conditions provided' do target(Dragon, target_type).min(:toes_on_claw, :is_fire_breathing => true).should == 4 target(Dragon, target_type).min(:toes_on_claw, :is_fire_breathing => false).should == 3 @@ -174,6 +196,21 @@ def target(klass, target_type) target(Country, target_type).max(:gold_reserve_value).should == BigDecimal('22589877164500.0') end + it 'should provide the highest value of a DateTime property' do + target(Dragon, target_type).min(:birth_at).should be_kind_of(DateTime) + target(Dragon, target_type).min(:birth_at).to_s.should == @birth_at.to_s + end + + it 'should provide the highest value of a Date property' do + target(Dragon, target_type).min(:birth_on).should be_kind_of(Date) + target(Dragon, target_type).min(:birth_on).to_s.should == @birth_on.to_s + end + + it 'should provide the highest value of a Time property' do + target(Dragon, target_type).min(:birth_time).should be_kind_of(Time) + target(Dragon, target_type).min(:birth_time).to_s.should == @birth_time.to_s + end + it 'should provide the highest value when conditions provided' do target(Dragon, target_type).max(:toes_on_claw, :is_fire_breathing => true).should == 5 target(Dragon, target_type).max(:toes_on_claw, :is_fire_breathing => false).should == 3 @@ -247,6 +284,28 @@ def target(klass, target_type) end end end + + describe ".aggregate on a #{target_type}" do + describe 'with no arguments' do + it 'should raise an error' do + lambda { target(Dragon, target_type).aggregate }.should raise_error(ArgumentError) + end + end + + describe 'with only aggregate fields specified' do + it 'should provide aggregate results' do + results = target(Dragon, target_type).aggregate(:all.count, :name.count, :toes_on_claw.min, :toes_on_claw.max, :toes_on_claw.avg, :toes_on_claw.sum) + results.should == [ 3, 2, 3, 5, 4.0, 12 ] + end + end + + describe 'with aggregate fields and a property to group by' do + it 'should provide aggregate results' do + results = target(Dragon, target_type).aggregate(:all.count, :name.count, :toes_on_claw.min, :toes_on_claw.max, :toes_on_claw.avg, :toes_on_claw.sum, :is_fire_breathing) + results.should == [ [ 1, 1, 3, 3, 3.0, 3, false ], [ 2, 1, 4, 5, 4.5, 9, true ] ] + end + end + end end end end From c1028ea680cef1dcfb1aa9bafd65bc46eb141cde Mon Sep 17 00:00:00 2001 From: Dan Kubb Date: Tue, 15 Jul 2008 12:48:45 -0700 Subject: [PATCH 03/10] Fixed failing spec in merb_datamapper --- merb_datamapper/spec/connection_spec.rb | 16 ++++++++-------- merb_datamapper/spec/spec_helper.rb | 5 +++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/merb_datamapper/spec/connection_spec.rb b/merb_datamapper/spec/connection_spec.rb index c90ac0c5..cef5a390 100644 --- a/merb_datamapper/spec/connection_spec.rb +++ b/merb_datamapper/spec/connection_spec.rb @@ -11,14 +11,14 @@ config = { 'development' => { - 'adapter' => 'myadapter', - 'more_stuff' => 'more_stuff', - 'repositories' => { - 'repo1' => { - 'adapter' => 'mysql' - } - } - } + 'adapter' => 'myadapter', + 'more_stuff' => 'more_stuff', + 'repositories' => { + 'repo1' => { + 'adapter' => 'mysql', + }, + }, + }, } Merb::Orms::DataMapper.should_receive(:full_config).once.and_return(config) diff --git a/merb_datamapper/spec/spec_helper.rb b/merb_datamapper/spec/spec_helper.rb index 3c4c0ad6..e74c0477 100644 --- a/merb_datamapper/spec/spec_helper.rb +++ b/merb_datamapper/spec/spec_helper.rb @@ -1,6 +1,11 @@ $TESTING=true $:.push File.join(File.dirname(__FILE__), '..', 'lib') +require 'rubygems' + +gem 'dm-core', '=0.9.3' +require 'dm-core' + module Merb module Plugins def self.config From 98bbb7a705ee54ec0291c0fec7afa1a297c0945a Mon Sep 17 00:00:00 2001 From: Wesley Beary Date: Tue, 15 Jul 2008 16:35:32 -0700 Subject: [PATCH 04/10] fix views so that options will be passed along correctly --- adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb | 11 ++++++++++- adapters/dm-couchdb-adapter/lib/couchdb_views.rb | 14 +++++--------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb index c38baedd..afd37c3b 100644 --- a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb +++ b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb @@ -228,7 +228,16 @@ def view_request(query) def query_string(query) query_string = [] - query_string << "key=%22#{query.key}%22" if query.key + if query.view_options + query_string += + query.view_options.map do |key, value| + if [:endkey, :key, :startkey].include? key + URI.escape(%Q(#{key}="#{value}")) + else + URI.escape("#{key}=#{value}") + end + end + end query_string << "count=#{query.limit}" if query.limit query_string << "descending=#{query.add_reversed?}" if query.add_reversed? query_string << "skip=#{query.offset}" if query.offset diff --git a/adapters/dm-couchdb-adapter/lib/couchdb_views.rb b/adapters/dm-couchdb-adapter/lib/couchdb_views.rb index 85e05ae1..99e263f7 100644 --- a/adapters/dm-couchdb-adapter/lib/couchdb_views.rb +++ b/adapters/dm-couchdb-adapter/lib/couchdb_views.rb @@ -1,6 +1,6 @@ module DataMapper class Query - attr_accessor :view, :key + attr_accessor :view, :view_options end end @@ -18,18 +18,14 @@ def initialize(model, name) def create_getter @model.class_eval <<-EOS, __FILE__, __LINE__ def self.#{@name}(*args) + options = {} if args.size == 1 && args.last.is_a?(String) - options = {} - key = args.shift - elsif args.empty? - options = {} - key = nil + options[:key] = args.shift else options = args.pop - key = options.delete(:key) end - query = Query.new(repository, self, options) - query.key = key + query = Query.new(repository, self) + query.view_options = options query.view = '#{@name}' if options.is_a?(Hash) && options.has_key?(:repository) repository(options.delete(:repository)).read_many(query) From dfd7e4e17d62511cd1f737a8b66d0543e7262f0e Mon Sep 17 00:00:00 2001 From: Wesley Beary Date: Tue, 15 Jul 2008 17:27:27 -0700 Subject: [PATCH 05/10] should use put instead of post on create if the user has specified an id --- adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb | 9 ++++++++- adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb | 8 ++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb index afd37c3b..5819d652 100644 --- a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb +++ b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb @@ -50,7 +50,14 @@ def escaped_db_name def create(resources) created = 0 resources.each do |resource| - result = http_post("/#{self.escaped_db_name}", resource.to_json(true)) + key = resource.class.key(self.name).map do |property| + resource.instance_variable_get(property.instance_variable_name) + end + if key.compact.empty? + result = http_post("/#{self.escaped_db_name}", resource.to_json(true)) + else + result = http_put("/#{self.escaped_db_name}/#{key}", resource.to_json(true)) + end if result["ok"] key = resource.class.key(self.name) if key.size == 1 diff --git a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb index eaf820f7..3899faa7 100644 --- a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb +++ b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb @@ -73,6 +73,14 @@ class Company company.id.should_not == nil end + it "should create a record with a specified id" do + user_with_id = new_user + user_with_id.id = 'user_id' + user_with_id.save.should == true + User.get!('user_id').should == user_with_id + user_with_id.destroy + end + it "should get a record" do pending("No CouchDB connection.") if @no_connection created_user = new_user From 7b3534609115866bf0fd91e987798930f1202619 Mon Sep 17 00:00:00 2001 From: Wesley Beary Date: Tue, 15 Jul 2008 19:39:57 -0700 Subject: [PATCH 06/10] should be pending rather than fail the test if there is no couchdb available --- adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb index 3899faa7..b68813e9 100644 --- a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb +++ b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb @@ -74,6 +74,7 @@ class Company end it "should create a record with a specified id" do + pending("No CouchDB connection.") if @no_connection user_with_id = new_user user_with_id.id = 'user_id' user_with_id.save.should == true From b563cea4fcba0f805a5b8f42ba22c7659bc3189b Mon Sep 17 00:00:00 2001 From: Wesley Beary Date: Tue, 15 Jul 2008 19:45:36 -0700 Subject: [PATCH 07/10] change the way keys are encoded as it was mucking up complex keys (hopefully fixed), also removed the skip=0 that kept being added/overriding skip values that were set --- adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb index 5819d652..8101414e 100644 --- a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb +++ b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb @@ -239,7 +239,7 @@ def query_string(query) query_string += query.view_options.map do |key, value| if [:endkey, :key, :startkey].include? key - URI.escape(%Q(#{key}="#{value}")) + URI.escape(%Q(#{key}=#{value.to_json})) else URI.escape("#{key}=#{value}") end @@ -247,7 +247,8 @@ def query_string(query) end query_string << "count=#{query.limit}" if query.limit query_string << "descending=#{query.add_reversed?}" if query.add_reversed? - query_string << "skip=#{query.offset}" if query.offset + query_string << "skip=#{query.offset}" if query.offset != 0 + p query_string.empty? ? nil : "?#{query_string.join('&')}" query_string.empty? ? nil : "?#{query_string.join('&')}" end From 9537ec0b1d145a63e63c026cab7ba920270eea94 Mon Sep 17 00:00:00 2001 From: Wesley Beary Date: Tue, 15 Jul 2008 19:47:20 -0700 Subject: [PATCH 08/10] remove a print string I was debugging with --- adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb index 8101414e..0aa2db76 100644 --- a/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb +++ b/adapters/dm-couchdb-adapter/lib/couchdb_adapter.rb @@ -214,7 +214,7 @@ def ad_hoc_request(query) request.body = %Q({"map": "function(doc) { - if (doc.type == '#{query.model.name.downcase}' && #{conditions.join(" && ")}) { + if (doc.type == '#{query.model.name.downcase}' && #{conditions.join(" && ")}) { emit(#{key}, doc); } }" @@ -232,7 +232,7 @@ def view_request(query) "#{query_string(query)}" request = Net::HTTP::Get.new(uri) end - + def query_string(query) query_string = [] if query.view_options @@ -248,7 +248,6 @@ def query_string(query) query_string << "count=#{query.limit}" if query.limit query_string << "descending=#{query.add_reversed?}" if query.add_reversed? query_string << "skip=#{query.offset}" if query.offset != 0 - p query_string.empty? ? nil : "?#{query_string.join('&')}" query_string.empty? ? nil : "?#{query_string.join('&')}" end From aff72b4c389b5a37dfc5e8845e8a535d007e88c5 Mon Sep 17 00:00:00 2001 From: Wesley Beary Date: Tue, 15 Jul 2008 22:55:47 -0700 Subject: [PATCH 09/10] fix views to take non-string keys, as integers and arrays(for complex keys) should be valid as a first parameter, specifying :key => key should work as before, added spec to test --- adapters/dm-couchdb-adapter/README | 2 +- adapters/dm-couchdb-adapter/lib/couchdb_views.rb | 2 +- adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb | 8 +++++++- adapters/dm-couchdb-adapter/spec/couchdb_view_spec.rb | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/adapters/dm-couchdb-adapter/README b/adapters/dm-couchdb-adapter/README index 6f3b4c10..c35a0068 100644 --- a/adapters/dm-couchdb-adapter/README +++ b/adapters/dm-couchdb-adapter/README @@ -24,4 +24,4 @@ class User view :by_name, "function(doc) { if (doc.type == 'user') map(doc.name, doc) }" end -You could then call User.by_name to get a listing of users ordered by name, or pass a key to try and find a specific user by their name, ie User.by_name(:key => 'username'). \ No newline at end of file +You could then call User.by_name to get a listing of users ordered by name, or pass a key to try and find a specific user by their name, ie User.by_name(:key => 'username'). diff --git a/adapters/dm-couchdb-adapter/lib/couchdb_views.rb b/adapters/dm-couchdb-adapter/lib/couchdb_views.rb index 99e263f7..fbfc1854 100644 --- a/adapters/dm-couchdb-adapter/lib/couchdb_views.rb +++ b/adapters/dm-couchdb-adapter/lib/couchdb_views.rb @@ -19,7 +19,7 @@ def create_getter @model.class_eval <<-EOS, __FILE__, __LINE__ def self.#{@name}(*args) options = {} - if args.size == 1 && args.last.is_a?(String) + if args.size == 1 && !args.is_a?(Hash) options[:key] = args.shift else options = args.pop diff --git a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb index b68813e9..65902b6f 100644 --- a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb +++ b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb @@ -74,7 +74,7 @@ class Company end it "should create a record with a specified id" do - pending("No CouchDB connection.") if @no_connection + pending("No CouchDB connection.") if @no_connection user_with_id = new_user user_with_id.id = 'user_id' user_with_id.save.should == true @@ -197,6 +197,12 @@ class Company User.by_age.first.should == User.all(:order => [:age]).first end + it "should be able to call stored views with keys" do + pending("No CouchDB connection.") if @no_connection + User.by_name("Aaron").first == User.all(:name => "Aaron").first + User.by_age(30).first == User.all(:age => 30).first + end + def create_procedures DataMapper.auto_migrate! end diff --git a/adapters/dm-couchdb-adapter/spec/couchdb_view_spec.rb b/adapters/dm-couchdb-adapter/spec/couchdb_view_spec.rb index b02d469a..c6a3657e 100644 --- a/adapters/dm-couchdb-adapter/spec/couchdb_view_spec.rb +++ b/adapters/dm-couchdb-adapter/spec/couchdb_view_spec.rb @@ -37,4 +37,4 @@ def self.default_repository_name Zoo.view :open Zoo.should respond_to(:open) end -end \ No newline at end of file +end From 8a0846236902fff1ad38291de8dead0c041c027d Mon Sep 17 00:00:00 2001 From: Wesley Beary Date: Tue, 15 Jul 2008 22:57:11 -0700 Subject: [PATCH 10/10] add tests to make sure that passing the key as the first parameter to a view and spelling out the key in a hash get the same result --- adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb index 65902b6f..4502fbfd 100644 --- a/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb +++ b/adapters/dm-couchdb-adapter/spec/couchdb_adapter_spec.rb @@ -201,6 +201,8 @@ class Company pending("No CouchDB connection.") if @no_connection User.by_name("Aaron").first == User.all(:name => "Aaron").first User.by_age(30).first == User.all(:age => 30).first + User.by_name("Aaron").first == User.by_name(:key => "Aaron").first + User.by_age(30).first == User.by_age(:key => 30).first end def create_procedures