recordMethod() in fflib_MethodCountRecorder fails if methodArgs contains a mock instance with equals(Object o) method overridden

[donnie745]

When an fflib generated mock class implements an interface with equals/hashcode, there is an issue with recording calls to equals with an argument being an instance of such a mock type.

In this scenario the methodCountByArgs.get(methodArg) call within the fflib_MethodCountRecorder class' recordMethod() method calls the overridden equals method in fflib_MethodArgValues. This in turn then calls the mock instance's equals method which then results in a call back to the fflib_MethodCountRecorder recordMethod() method once again; we then go back through the same method calls as before and end up in an infinite cycle of method calls cycling between the fflib_MethodArgValues equals() and the mock class' equals() methods (till an exception eventually gets thrown).

Perhaps the fflib_MethodArgValues class could do something clever to wrap mock objects and have wrapper implementing equals using referential === for mocked instances to avoid this problem (would need the code generator to make the generated mock classes implement some no-method fflib interface to identify the instances as being an fflib generated mock class for this I imagine).

[afawcett]

Thanks @donnie745, marked this as an enhancement, also a good fix suggestion thanks!

[dfruddffdc]

Hi, sorry for the delay on this! I understand your use case, but I suspect it could introduce regressions for existing tests. For example:

Account initial = new Account(Name='Before');
Account modified = new Account(Name='After');
MyClass.doStuff(initial); //modifies the account name, then calls MyOtherClass.doAccountThings
((IMyOtherClass)mocks.verify(myOtherClass)).doAccountThings(modified);

We must therefore continue to match elements that are equivalent (==) even if they are not referentially equal (===).

With all that said, I think it's a solid suggestion and we should implement it (cautiously).

[donnie745]

Hi there, been a while since I first raised this but we're looking to upgrade our fflib apex mocks library from our old version that included some of our own bespoke modifications to fix this issue (which followed my original suggested solution above whereby a wrapper class was created with an equals override using == on wrapped non-mock objects or mock objects not overriding equals() and === on wrapped mock objects that override the equals() method (which we made identifiable by making such mock classes implement a new IApexMockWithEqualsOverride interface we defined purely for identifying objects as an fflib mock with overridden equals() implementation on the mock).

I'll maybe dig into the code of the new version of the library next week but I was wondering if you might be able to tell me whether this issue is still in the latest version of the library (which the open status of this ticket might suggest is the case) to save me some time investigating this:-)

Thanks,
Donnie