3.2.0: Regression in `.where`?

[nialbima]

Hiya!

SYSTEM INFO: Rails: 6.1.7.1 Ruby: 3.0.4

Old version of ActiveHash: 3.1.1 New version: 3.2.0

I'm working on updating our version of ActiveHash to 3.2 from 3.1.1. We use this gem extensively because it's great, and it's usually this (example is not direct from our app, but is structured the same way):

class Disposition < ActiveHash::Base
  include ActiveHash::Enum

  enum_accessor :name
  self.data = [
    {
      id: 1,
      name: :a_disposition,
      display: true,
    },
    {
      id: 2,
      name: :another_disposition,
      display: false,
    },
  ]
end

We have a lot of code that uses .where to filter our dictionaries: Disposition.where(display: true). This is still locating the correct records, but some of the changes to .where in 3.2.0 seem to have broken our ability to call .map(&:name) on those records. I'm expecting to see an array of names associated with our dictionary, and am getting this as output:

#<Disposition:0x00000001418dd4b0>#<Disposition:0x00000001418dcab0>#<Disposition:0x00000001418dc5b0>#<Disposition:0x00000001418d7ec0>#<Disposition:0x00000001418d7858>#<Disposition:0x00000001418d71c8>#<Disposition:0x00000001418d6c50>=> []

I can get the expected list by going via .all or calling .all_records on the output, but I was surprised to see this breaking on us during my upgrade.

Is this a bug? Did something change here?

[kbrock]

The way relations are created changed quite a bit to follow rails 7.1 trends. So a bug like there (where it is probably not delegating correctly) makes sense.

Using an active record 6.1 model, this is the functionality I get:

irb(main):001:0> VmOrTemplate.all.spawn.class
=> VmOrTemplate::ActiveRecord_Relation
irb(main):002:0> VmOrTemplate.all.class
=> VmOrTemplate::ActiveRecord_Relation
irb(main):003:0> VmOrTemplate.where(:id => 1).class
=> VmOrTemplate::ActiveRecord_Relation

Are you seeing similar behavior for your active hash model? If you throw map onto all 3 of the above, do they work for you?

I'm swamped on work right now. Anyone else able to take a peek at this?

[nialbima]

@kbrock if somebody can point me in the right direction, I can take a stab at figuring it out.

[flavorjones]

I'm investigating now!

[flavorjones]

Ok, the problem here is that the attribute :display collides with Object#display.

Specifically, ActiveHash::Relation::Condition uses public_send to send a message by the name of the constraint key, and checks the result against the constraint value:

      constraints.send(expectation_method) do |attribute, expected|
        value = record.public_send(attribute)

        matches_value?(value, expected)
      end

For both objects, the value is nil (so neither matches the display: true constraint) and the response is an empty array, [].

Unfortunately, Object#display has a side effect of emitting information to STDOUT which makes the display garbled. If the output is this:

#<Disposition:0x00000001418dd4b0>#<Disposition:0x00000001418dcab0>#<Disposition:0x00000001418dc5b0>#<Disposition:0x00000001418d7ec0>#<Disposition:0x00000001418d7858>#<Disposition:0x00000001418d71c8>#<Disposition:0x00000001418d6c50>=> []

that's actually seven Disposition objects having #display called on them, followed by the irb prompt => followed by the actual result, [].

So that's the diagnosis, anyway. Not sure about solutions right now.

[flavorjones]

Here's a failing test that I'm working off of:

  describe "colliding methods" do
    it "should handle attributes named after existing methods" do
      klass = Class.new(ActiveHash::Base) do
        self.data = [
          {
            id: 1,
            name: "Aaa",
            display: true,
          },
          {
            id: 2,
            name: "Bbb",
            display: false,
          },
        ]
      end
      expect(klass.where(display: true).length).to eq(1)
    end
  end

[flavorjones]

I started to try to make ActiveHash::Base subclass BasicObject but it's turning into quite an invasive change. Other creative ideas would be welcome, I think.

[flavorjones]

I've submitted a PR at #281, which is a simple fix -- I'd love feedback on whether that's sufficient (or whether it will break untested use cases).