Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Support for custom Relation classes #1665

Closed
wants to merge 3 commits into from

3 participants

@reu

Just extracted the creation of the Relation object to a separated method, so we can easily use a subclassed relation.

@pixeltrix
Owner

Tests? I'm assuming you want to be able to create a subclass of Relation something like this:

class MyRelation < ActiveRecord::Relation
  def order(*args)
    # custom ordering
  end
end

class Product < ActiveRecord::Base
  private
  def create_relation
    MyRelation.new(self, arel_table)
  end
end

If that's what you want then you need to have tests for this functionality and that any spawned relations are the same subclass.

@reu

Yeah, exactly that. These were just some minor refactoring that I assumed that could already be merged. Anyway, I will write some tests for relation subclasses, don't know for sure if it is a good/useful idea tough.

@pixeltrix
Owner

If you don't have tests then there's no guarantee it won't be broken - even by a patch release.

@reu

Don't know this is the best way to test this, but here goes a first try.

@isaacsanders

Is this still an issue @reu?

@reu

Wow, actually I didn't even remember this... must take a look at the current state of AR, but I don't think it is valid anymore.

Anyway, it was never a issue, it was just a small refactoring. Gonna close this for now.

@reu reu closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
11 activerecord/lib/active_record/base.rb
@@ -657,11 +657,8 @@ def reset_sequence_name #:nodoc:
# set_table_name "project"
# end
def set_table_name(value = nil, &block)
- @quoted_table_name = nil
+ @quoted_table_name = @arel_table = @relation = nil
define_attr_method :table_name, value, &block
- @arel_table = nil
-
- @relation = Relation.new(self, arel_table)
end
alias :table_name= :set_table_name
@@ -974,7 +971,7 @@ def instantiate(record)
private
def relation #:nodoc:
- @relation ||= Relation.new(self, arel_table)
+ @relation ||= create_relation
if finder_needs_type_condition?
@relation.where(type_condition).create_with(inheritance_column.to_sym => sti_name)
@@ -983,6 +980,10 @@ def relation #:nodoc:
end
end
+ def create_relation
+ Relation.new(self, arel_table)
+ end
+
def find_sti_class(type_name)
if type_name.blank? || !columns_hash.include?(inheritance_column)
self
View
76 activerecord/test/cases/custom_relation_test.rb
@@ -0,0 +1,76 @@
+require "cases/helper"
+require "models/secure_person"
+
+class CustomRelationTest < ActiveRecord::TestCase
+ def test_scope_returns_the_custom_relation
+ assert_equal SecurePerson.scoped.class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_where
+ assert_equal SecurePerson.where(:name => "test").class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_select
+ assert_equal SecurePerson.select(:name).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_group
+ assert_equal SecurePerson.group(:parent).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_order
+ assert_equal SecurePerson.order(:name).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_except
+ assert_equal SecurePerson.except(:name).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_reorder
+ assert_equal SecurePerson.reorder(:name).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_limit
+ assert_equal SecurePerson.limit(5).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_offset
+ assert_equal SecurePerson.offset(5).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_joins
+ assert_equal SecurePerson.joins(:parent).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_where
+ assert_equal SecurePerson.where(:name => "test").class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_preload
+ assert_equal SecurePerson.preload(:parent).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_eager_load
+ assert_equal SecurePerson.eager_load(:parent).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_includes
+ assert_equal SecurePerson.includes(:parent).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_from
+ assert_equal SecurePerson.from(:secure_people).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_lock
+ assert_equal SecurePerson.lock(:name).class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_readonly
+ assert_equal SecurePerson.readonly.class, EncryptedRelation
+ end
+
+ def test_spawns_the_custom_relation_on_having
+ assert_equal SecurePerson.having(:name => "test").class, EncryptedRelation
+ end
+end
View
22 activerecord/test/models/secure_person.rb
@@ -0,0 +1,22 @@
+class SecurePerson < ActiveRecord::Base
+ belongs_to :parent, :class_name => "SecurePerson"
+
+ before_save :encrypt_name
+
+ private
+
+ def encrypt_name
+ self.name = name.reverse
+ end
+
+ def self.create_relation
+ EncryptedRelation.new(self, arel_table)
+ end
+end
+
+class EncryptedRelation < ActiveRecord::Relation
+ def build_where(opts, *args)
+ opts[:name] = opts[:name].reverse if opts[:name]
+ super
+ end
+end
View
5 activerecord/test/schema/schema.rb
@@ -709,6 +709,11 @@ def create_table(*args, &block)
t.string 'a$b'
end
+ create_table :secure_people, :force => true do |t|
+ t.integer :parent_id
+ t.string :name
+ end
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
Something went wrong with that request. Please try again.