Skip to content

Commit

Permalink
between_times one-sided and Array args (#63)
Browse files Browse the repository at this point in the history
* - Support Array argument to between_times
- Cleanup specs

* `#between_times` now accepts one-sided arguments, e.g. `Time, nil` or `nil, Time`.
  • Loading branch information
johnnyshields committed May 28, 2016
1 parent f6970cf commit 85c3867
Show file tree
Hide file tree
Showing 13 changed files with 188 additions and 103 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
* BREAKING CHANGE - `#between` method has been removed for ActiveRecord (was deprecated in 2.2.0)
* BREAKING CHANGE - Mongoid `:order` option now requires a Hash arguments in the form { field: direction }, i.e. the same as `Mongoid::Document#order_by`.
* BREAKING CHANGE - Drop support for option `:year` used as a standalone. Use `by_year` instead.
* `#between_times now accepts a `Range` as an argument, while continuing to support existing `Time, Time` interface.
* `#between_times` now accepts one-sided arguments, e.g. `Time, nil` or `nil, Time`.
* `#between_times` now accepts `Range` or `Array` as an argument, while continuing to support existing `Time, Time` interface.
* Timespan "strict" query now sets double-sided constraints on both fields to ensure database indexes are used properly.
* Remove hash rocket syntax. Not considered breaking as Ruby 1.9.3 was already minimum supported version.

## v2.2.2 - Unreleased

Expand Down
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,15 @@ class MyModel
ByStar adds the following finder scopes (class methods) to your model to query time ranges.
These accept a `Date`, `Time`, or `DateTime` object as an argument, which defaults to `Time.zone.now` if not specified:

* `between_times(start_time, end_time)` Finds all records occurring between the two given times
* `before(end_time)` Finds all records occurring before the given time
* `after(start_time)` Finds all records occurring after the given time
* `between_times(start_time, end_time)` - Finds all records occurring between the two given times.
* `before(end_time)` - Finds all records occurring before the given time
* `after(start_time)` - Finds all records occurring after the given time

`between_times` supports alternate argument forms:
* `between_times(Range)`
* `between_times(Array)`
* `between_times(start_time, nil)` - same as `after(start_time)`
* `between_times(nil, end_time)` - same as `before(end_time)`

### Time Range Scopes

Expand Down
25 changes: 15 additions & 10 deletions lib/by_star/between.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,29 @@ module Between
def between_times(*args)
options = args.extract_options!.symbolize_keys!

# Do not apply default offset here
start_time, end_time = case args[0]
when Array, Range then [args[0].first, args[0].last]
else args[0..1]
end

offset = (options[:offset] || 0).seconds
range = if args[0].is_a?(Range)
(args[0].first + offset)..(args[0].last + offset)
else
(args[0] + offset)..(args[1] + offset)
end
start_time += offset if start_time
end_time += offset if end_time

start_field = by_star_start_field(options)
end_field = by_star_end_field(options)
scope = by_star_scope(options)

scope = if start_field == end_field
by_star_point_query(scope, start_field, range)
scope = if !end_time

This comment has been minimized.

Copy link
@spdawson

spdawson May 28, 2016

It would be handy if the !start_time && !end_time case were handled here too, with scope returned unaltered in this case.

by_star_after_query(scope, start_field, start_time)
elsif !start_time
by_star_before_query(scope, start_field, end_time)
elsif start_field == end_field
by_star_point_query(scope, start_field, start_time, end_time)
elsif options[:strict]
by_star_span_strict_query(scope, start_field, end_field, range)
by_star_span_strict_query(scope, start_field, end_field, start_time, end_time)
else
by_star_span_overlap_query(scope, start_field, end_field, range, options)
by_star_span_overlap_query(scope, start_field, end_field, start_time, end_time, options)
end

scope = by_star_order(scope, options[:order]) if options[:order]
Expand Down
8 changes: 6 additions & 2 deletions lib/by_star/directional.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@ module Directional

def before(*args)
with_by_star_options(*args) do |time, options|
scope = by_star_scope(options)
field = by_star_start_field(options)
time = ByStar::Normalization.time(time)
before_query(time, options)
by_star_before_query(scope, field, time)
end
end
alias_method :before_now, :before

def after(*args)
with_by_star_options(*args) do |time, options|
scope = by_star_scope(options)
field = by_star_start_field(options)
time = ByStar::Normalization.time(time)
after_query(time, options)
by_star_after_query(scope, field, time)
end
end
alias_method :after_now, :after
Expand Down
30 changes: 14 additions & 16 deletions lib/by_star/orm/active_record/by_star.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,32 @@ module ClassMethods

protected

def by_star_point_query(scope, field, range)
scope.where("#{field} >= ? AND #{field} <= ?", range.first, range.last)
def by_star_default_field
"#{self.table_name}.created_at"
end

def by_star_span_strict_query(scope, start_field, end_field, range)
scope.where("#{start_field} >= ? AND #{start_field} <= ? AND #{end_field} >= ? AND #{end_field} <= ?", range.first, range.last, range.first, range.last)
def by_star_point_query(scope, field, start_time, end_time)
scope.where("#{field} >= ? AND #{field} <= ?", start_time, end_time)
end

def by_star_span_overlap_query(scope, start_field, end_field, range, options)
scope.where("#{end_field} > ? AND #{start_field} < ?", range.first, range.last)
def by_star_span_strict_query(scope, start_field, end_field, start_time, end_time)
scope.where("#{start_field} >= ? AND #{start_field} <= ? AND #{end_field} >= ? AND #{end_field} <= ?", start_time, end_time, start_time, end_time)
end

def by_star_order(scope, order)
scope.order(order)
def by_star_span_overlap_query(scope, start_field, end_field, start_time, end_time, options)
scope.where("#{end_field} > ? AND #{start_field} < ?", start_time, end_time)
end

def by_star_default_field
"#{self.table_name}.created_at"
def by_star_before_query(scope, field, time)
scope.where("#{field} <= ?", time)
end

def before_query(time, options={})
field = by_star_start_field(options)
by_star_scope(options).where("#{field} <= ?", time)
def by_star_after_query(scope, field, time)
scope.where("#{field} >= ?", time)
end

def after_query(time, options={})
field = by_star_start_field(options)
by_star_scope(options).where("#{field} >= ?", time)
def by_star_order(scope, order)
scope.order(order)
end

def oldest_query(options={})
Expand Down
24 changes: 12 additions & 12 deletions lib/by_star/orm/mongoid/by_star.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,30 @@ def by_star_default_field
:created_at
end

def by_star_point_query(scope, field, range)
def by_star_point_query(scope, field, start_time, end_time)
range = start_time..end_time
scope.where(field => range)
end

def by_star_span_strict_query(scope, start_field, end_field, range)
def by_star_span_strict_query(scope, start_field, end_field, start_time, end_time)
range = start_time..end_time
scope.where(start_field => range).where(end_field => range)
end

def by_star_span_overlap_query(scope, start_field, end_field, range, options)
scope.gt(end_field => range.first).lt(start_field => range.last)
def by_star_span_overlap_query(scope, start_field, end_field, start_time, end_time, options)
scope.gt(end_field => start_time).lt(start_field => end_time)
end

def by_star_order(scope, order)
scope.order_by(order)
def by_star_before_query(scope, field, time)
scope.lte(field => time)
end

def before_query(time, options={})
field = by_star_start_field(options)
by_star_scope(options).lte(field => time)
def by_star_after_query(scope, field, time)
scope.gte(field => time)
end

def after_query(time, options={})
field = by_star_start_field(options)
by_star_scope(options).gte(field => time)
def by_star_order(scope, order)
scope.order_by(order)
end

def oldest_query(options={})
Expand Down
27 changes: 4 additions & 23 deletions spec/integration/active_record/active_record_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
ActiveRecord::Base.logger = Logger.new(File.dirname(__FILE__) + '/../../tmp/activerecord.log')
end

it_behaves_like 'between_times'
it_behaves_like 'offset parameter'
it_behaves_like 'order parameter'
it_behaves_like 'scope parameter'
it_behaves_like 'by day'
it_behaves_like 'by direction'
it_behaves_like 'by fortnight'
Expand All @@ -31,27 +35,4 @@
it_behaves_like 'by weekend'
it_behaves_like 'by year'
it_behaves_like 'relative'
it_behaves_like 'offset parameter'
it_behaves_like 'scope parameter'

describe '#between_times' do
subject { Post.between_times(Time.zone.parse('2014-01-01'), Time.zone.parse('2014-01-06')) }
it { expect be_a(ActiveRecord::Relation) }
it { expect(subject.count).to eql(3) }

context ':order option' do

it 'should be able to order the result set asc' do
scope = Post.by_year(Time.zone.now.year, order: 'created_at ASC')
expect(scope.order_values).to eq ['created_at ASC']
expect(scope.first.created_at).to eq Time.zone.parse('2014-01-01 17:00:00')
end

it 'should be able to order the result set desc' do
scope = Post.by_year(Time.zone.now.year, order: 'created_at DESC')
expect(scope.order_values).to eq ['created_at DESC']
expect(scope.first.created_at).to eq Time.zone.parse('2014-04-15 17:00:00')
end
end
end
end if testing_active_record?
34 changes: 4 additions & 30 deletions spec/integration/mongoid/mongoid_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
Mongoid.purge!
end

it_behaves_like 'between_times'
it_behaves_like 'offset parameter'
it_behaves_like 'order parameter'
it_behaves_like 'scope parameter'
it_behaves_like 'by day'
it_behaves_like 'by direction'
it_behaves_like 'by fortnight'
Expand All @@ -30,34 +34,4 @@
it_behaves_like 'by weekend'
it_behaves_like 'by year'
it_behaves_like 'relative'
it_behaves_like 'offset parameter'
it_behaves_like 'scope parameter'

describe '#between_times' do
subject { Post.between_times(Time.zone.parse('2014-01-01'), Time.zone.parse('2014-01-06')) }
it { should be_a(Mongoid::Criteria) }
it { expect(subject.count).to eql(3) }

context ':order option' do

it 'should be able to order the result set asc' do
scope = Post.by_year(Time.zone.now.year, order: {created_at: :asc})
expect(scope.options[:sort]).to eq({'created_at' => 1})
expect(scope.first.created_at).to eq Time.zone.parse('2014-01-01 17:00:00')
end

it 'should be able to order the result set desc' do
scope = Post.by_year(Time.zone.now.year, order: {created_at: :desc})
expect(scope.options[:sort]).to eq({'created_at' => -1})
expect(scope.first.created_at).to eq Time.zone.parse('2014-04-15 17:00:00')
end
end
end

describe '#between' do
it 'should not override Mongoid between method' do
posts = Post.between(created_at: Time.zone.parse('2014-01-01')..Time.zone.parse('2014-01-06'))
expect(posts.count).to eql(3)
end
end
end if testing_mongoid?
67 changes: 67 additions & 0 deletions spec/integration/shared/between_times.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
require 'spec_helper'

shared_examples_for 'between_times' do

describe '#between_times' do
subject { Post.between_times(Time.zone.parse('2014-01-01'), Time.zone.parse('2014-01-06')) }

if testing_active_record?
it { is_expected.to be_a(ActiveRecord::Relation) }
else testing_mongoid?
it { is_expected.to be_a(Mongoid::Criteria) }
end

it { expect(subject.count).to eql(3) }

context 'one-sided query' do

context 'point query' do

context 'only start time' do
subject { Post.between_times(Time.zone.parse('2014-01-01'), nil) }
it { expect(subject.count).to eql(12) }
end

context 'only end time' do
subject { Post.between_times(nil, Time.zone.parse('2014-01-01')) }
it { expect(subject.count).to eql(10) }
end
end

context 'timespan loose query' do

context 'only start time' do
subject { Event.between_times(Time.zone.parse('2014-01-01'), nil, strict: false) }
it { expect(subject.count).to eql(9) }
end

context 'only end time' do
subject { Event.between_times(nil, Time.zone.parse('2014-01-01'), strict: false) }
it { expect(subject.count).to eql(13) }
end
end

context 'timespan strict query' do

context 'only start time' do
subject { Event.between_times(Time.zone.parse('2014-01-01'), nil) }
it { expect(subject.count).to eql(9) }
end

context 'only end time' do
subject { Event.between_times(nil, Time.zone.parse('2014-01-01')) }
it { expect(subject.count).to eql(13) }
end
end
end
end

if testing_mongoid?
describe '#between' do
it 'should not override Mongoid between method' do
posts = Post.between(created_at: Time.zone.parse('2014-01-01')..Time.zone.parse('2014-01-06'))
expect(posts.count).to eql(3)
end
end
end
end
14 changes: 12 additions & 2 deletions spec/integration/shared/by_direction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
it { expect(subject.count).to eql(12) }
end

context 'timespan' do
context 'timespan default' do
subject { Event.before(Time.zone.parse '2014-01-05') }
it { expect(subject.count).to eql(13) }
end
Expand All @@ -19,6 +19,11 @@
it { expect(subject.count).to eql(13) }
end

context 'timespan not strict' do
subject { Event.before('2014-01-05', strict: false) }
it { expect(subject.count).to eql(13) }
end

context 'alternative field' do
subject { Event.before('2014-01-05', field: 'created_at') }
it { expect(subject.count).to eql(12) }
Expand Down Expand Up @@ -47,7 +52,7 @@
it { expect(subject.count).to eql(10) }
end

context 'timespan' do
context 'timespan default' do
subject { Event.after(Date.parse '2014-01-05') }
it { expect(subject.count).to eql(9) }
end
Expand All @@ -57,6 +62,11 @@
it { expect(subject.count).to eql(9) }
end

context 'timespan not strict' do
subject { Event.after('2014-01-05', strict: false) }
it { expect(subject.count).to eql(9) }
end

context 'alternative field' do
subject { Event.after('2014-01-05', field: 'created_at') }
it { expect(subject.count).to eql(10) }
Expand Down
3 changes: 2 additions & 1 deletion spec/integration/shared/offset_parameter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

shared_examples_for 'offset parameter' do

describe 'offset' do
describe ':offset' do

it 'should memoize the offset variable' do
expect(Event.instance_variable_get(:@by_star_offset)).to eq 3.hours
expect(Post.instance_variable_get(:@by_star_offset)).to be_nil
Expand Down
Loading

0 comments on commit 85c3867

Please sign in to comment.