Description
Ok, this one's kind of crazy, so bear with me...
When toying with a workaround to the problems caused by #509, I discovered a bug stemming from a bit of code that was in the original project's commit in 2009.
See, when commenting out the problematic magic model getter in #509, the execution path changed to something that has obviously not been used in the project in quite a while (or at least isn't tested and hasn't been reported). I was getting an error from PHP with the following message:
Illegal offset type in isset or empty
I traced this to a line that was attempting to use a model object as an array key in an isset call. Now, that method obviously expects a string
typed argument as a model class name, but for some reason was receiving an actual instance of a Model
object. Hmm, that isn't right... so I dug deeper.
Looking up the chain, I followed the code path to where a relationship was setting keys based on the results in a model load. Again this method seemed to be expecting a string
argument but was receiving a model instance. So I looked further up the chain. I quickly found that the culprit was a line in a private method that was passing a model instance directly to the set_keys()
method. All other similar instances of this type of call were first making sure to call get_class()
on the model instance, where this line wasn't.
Now, the confusing part was how these relationship builders were ever working. Curious, I dug deeper... Well, that was when I found that the whole reason this bug hasn't come up before is because this execution path was never actually invoked. Instead, when building associations, the normal execution path would go through the problematic dynamic getter and eventually would get reflected in a defensively coded private method that was making sure that any actual object instances were first inspected to get their string class name.
So yea, crazy bug all discovered by trying to circumvent the sketchy querying in #509. Luckily, a simple get_class()
in between the set_keys()
method not only fixed this bug, but also allowed #509 to be circumvented without any issue. :P
Crazy.