ActiveRecord's scope eql? not comparing properly #11947

Closed
brkattk opened this Issue Aug 20, 2013 · 8 comments

Comments

Projects
None yet
3 participants

brkattk commented Aug 20, 2013

Having an issue comparing two results. Talked to someone in the #rspec irc room and they mentioned the possibility that the scope doesn't implement eql properly. Point me in the right direction and I'll dig deeper, but I'm not sure where to begin here.

I've included (what I believe to be) the relevant code below

class Municipality < ActiveRecord::Base
  has_many :meter_groups
  has_many :users
  has_many :meters, through: :meter_groups

  validates_uniqueness_of :name

   ...

end
class MeterGroup < ActiveRecord::Base

  ...

  has_many :meters
  belongs_to :municipality

  validates_presence_of :municipality_id
  validates_presence_of :name

  ...

end
class DomainGuard
  attr_reader :chosen_municipality_id, :user, :subdomain

  def initialize(user, subdomain, chosen_municipality_id = nil)
    @user = user
    @subdomain = subdomain.downcase
    @chosen_municipality_id = chosen_municipality_id
  end

  def current_municipality
    chosen_municipality || subdomain_municipality
  end

  ...

  def meter_groups
    current_municipality ? MeterGroup.where(municipality_id: current_municipality.id) : MeterGroup.scoped
  end

  ...

private
  def chosen_municipality
    return false if non_admin_user? || chosen_municipality_id.blank?
    Municipality.find chosen_municipality_id
  end

  def subdomain_municipality
    if admin_user?
      Municipality.where("lower(subdomain) = ?", subdomain).first
    else
      Municipality.find user.municipality_id
    end
  end

  def admin_domain?
    subdomain == Subdomain::ADMIN_SUBDOMAIN
  end

  def excluded_domain?
    Subdomain::EXCLUDED_DOMAINS.include?(subdomain)
  end

  def admin_user?
    user.admin?
  end

  def non_admin_user?
    !admin_user?
  end
end  
require 'spec_helper'

describe DomainGuard do
  let(:municipality) { create(:municipality, subdomain: 'foobar') }
  let(:unassociated_municipality) { create(:municipality) }
  let(:meter_group) { create(:meter_group, municipality: municipality) }
  let(:unassociated_meter_group) { create(:meter_group, municipality: unassociated_municipality) }
  let(:meter) { create(:meter, meter_group: meter_group) }
  let(:unassociated_meter) { create(:meter, meter_group: unassociated_meter_group) }
  let(:parking_session) { create(:parking_session, meter: meter) }
  let(:unassociated_parking_session) { create(:parking_session, meter: unassociated_meter) }
  let(:violation) { create(:violation, parking_session: parking_session, meter: meter) }
  let(:unassociated_violation) { create(:violation, parking_session: unassociated_parking_session, meter: unassociated_meter) }
  let(:non_admin_user) { create(:user, role: 'moderator', municipality: municipality) }
  let(:admin_user) { create(:user, role: 'admin') }

  before(:each) do
    admin_user
    non_admin_user
    violation
    unassociated_violation
  end

  context "admin user" do

    ...

    context "on ADMIN_SUBDOMAIN" do

      ...

      context "with chosen_municipality_id specified" do
        subject { DomainGuard.new admin_user, Subdomain::ADMIN_SUBDOMAIN, municipality.id }

        ...

        its(:meter_groups) { should eql municipality.meter_groups }
        its(:meters) { should eql municipality.meters }
      end
    end
  ...
  end
end
1) DomainGuard admin user on ADMIN_SUBDOMAIN with chosen_municipality_id specified meter_groups
     Failure/Error: its(:meter_groups) { should eql municipality.meter_groups }

       expected: [#<MeterGroup id: 7, name: "Group1", created_at: "2013-08-20 17:41:36", updated_at: "2013-08-20 17:41:36", sys_default_language: nil, parking_init_grace_period: 0, parking_violation_grace_period: 0, parking_maximum_duration_minutes: 0, parking_nofine_cost: 0, parking_ticket_reduced_fine: 0, parking_ticket_full_fine: 0, phone_numbers: nil, send_sms_notifications: nil, access_point_ip: nil, municipality_id: 11, violation_tow_grace_period: 0, address_street_1: nil, address_street_2: nil, city: nil, state: nil, postal_code: nil>]
            got: [#<MeterGroup id: 7, name: "Group1", created_at: "2013-08-20 17:41:36", updated_at: "2013-08-20 17:41:36", sys_default_language: nil, parking_init_grace_period: 0, parking_violation_grace_period: 0, parking_maximum_duration_minutes: 0, parking_nofine_cost: 0, parking_ticket_reduced_fine: 0, parking_ticket_full_fine: 0, phone_numbers: nil, send_sms_notifications: nil, access_point_ip: nil, municipality_id: 11, violation_tow_grace_period: 0, address_street_1: nil, address_street_2: nil, city: nil, state: nil, postal_code: nil>]

       (compared using eql?)

       Diff:
     # ./spec/models/domain_guard_spec.rb:50:in `block (5 levels) in <top (required)>'

brkattk commented Aug 20, 2013

Just for clarity, the following specs pass

        it "returns the proper meter groups" do
          expect(subject.meter_groups.all).to eq municipality.meter_groups.all
        end
        it "returns the proper meters" do
          expect(subject.meters.all).to eq municipality.meters.all
        end
Owner

rafaelfranca commented Aug 20, 2013

If you want to compare two object you should not using eql? unless your object is a Hash.

From ruby documentation:

The eql? method returns true if obj and other refer to the same hash key. This is used by Hash to test members for equality. For objects of class Object, eql? is synonymous with ==. Subclasses normally continue this tradition by aliasing eql? to their overridden == method, but there are exceptions.

but there are exceptions it the important information here.

We can change Relation to alias eql? with == but I think it should already do it. Could you investigate?

brkattk commented Aug 20, 2013

I have also tried == and it yields the same results as eql?

1) DomainGuard admin user on ADMIN_SUBDOMAIN with chosen_municipality_id specified valid chosen_municipality_id meter_groups
     Failure/Error: its(:meter_groups) { should == municipality.meter_groups }
       expected: [#<MeterGroup id: 13, name: "Group1", created_at: "2013-08-20 18:59:47", updated_at: "2013-08-20 18:59:47", sys_default_language: nil, parking_init_grace_period: 0, parking_violation_grace_period: 0, parking_maximum_duration_minutes: 0, parking_nofine_cost: 0, parking_ticket_reduced_fine: 0, parking_ticket_full_fine: 0, phone_numbers: nil, send_sms_notifications: nil, access_point_ip: nil, municipality_id: 20, violation_tow_grace_period: 0, address_street_1: nil, address_street_2: nil, city: nil, state: nil, postal_code: nil>]
            got: [#<MeterGroup id: 13, name: "Group1", created_at: "2013-08-20 18:59:47", updated_at: "2013-08-20 18:59:47", sys_default_language: nil, parking_init_grace_period: 0, parking_violation_grace_period: 0, parking_maximum_duration_minutes: 0, parking_nofine_cost: 0, parking_ticket_reduced_fine: 0, parking_ticket_full_fine: 0, phone_numbers: nil, send_sms_notifications: nil, access_point_ip: nil, municipality_id: 20, violation_tow_grace_period: 0, address_street_1: nil, address_street_2: nil, city: nil, state: nil, postal_code: nil>] (using ==)
       Diff:
     # ./spec/models/domain_guard_spec.rb:51:in `block (6 levels) in <top (required)>'
Owner

rafaelfranca commented Aug 20, 2013

Could you try to print the to_sql of both scopes? I think it is because of this: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation.rb#L569

brkattk commented Aug 20, 2013

 => 51:           its(:meter_groups) { binding.pry; should eq municipality.meter_groups.all }
    52:           its(:meters) { should eq municipality.meters.all }
    53:         end
    54:
    55:         context "invalid chosen_municipality_id" do
    56:           subject { DomainGuard.new admin_user, Subdomain::ADMIN_SUBDOMAIN, 932883 }

[1] pry(#<RSpec::Core::ExampleGroup::Nested_1::Nested_1::Nested_2::Nested_6::Nested_1::Nested_4>)> subject.meter_groups.to_sql
=> "SELECT \"meter_groups\".* FROM \"meter_groups\"  WHERE \"meter_groups\".\"municipality_id\" = 44"
[2] pry(#<RSpec::Core::ExampleGroup::Nested_1::Nested_1::Nested_2::Nested_6::Nested_1::Nested_4>)> municipality.meter_groups.to_sql
=> "SELECT \"meter_groups\".* FROM \"meter_groups\"  WHERE \"meter_groups\".\"municipality_id\" = 44"

note that if I call .all on the municipality.meter_groups this spec passes...

This is a Rails 3.2 app don't think I mentioned that before

Owner

rafaelfranca commented Aug 20, 2013

Really weird. The code on 3-2-stable is the same

@brkattk brkattk added the stale label Apr 23, 2014

Owner

rafaelfranca commented May 1, 2014

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@rails-bot rails-bot closed this May 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment