apex-enterprise-patterns / fflib-apex-mocks

An Apex mocking framework for true unit testing in Salesforce, with Stub API support
BSD 3-Clause "New" or "Revised" License
423 stars 214 forks source link

methodRecorder not correct if methodArg in Map changes after recording #8

Closed jondavis9898 closed 8 years ago

jondavis9898 commented 8 years ago

The method recording uses a Object Map to store method arguments that were passed to a method so that verification can be performed. Since a method argument could be a reference to an Object (e.g. SObject, Apex class, etc.), only a shallow reference is maintained. Therefore, if the referenced object changes after the method was recorded, the recording is lost.

Example: Map<Object, Integer> myMap = new Map<Object, Integer>(); Account a = new Account(Name = 'foo'); myMap.put(a, 1); System.debug(myMap); // 17:39:27.13 (14714888)|USER_DEBUG|[4]|DEBUG|{Account:{Name=foo}=1} a.Name = 'bar'; System.debug(myMap); // 17:39:27.13 (14826544)|USER_DEBUG|[6]|DEBUG|{Account:{Name=bar}=null}

Unfortunately, APEX doesn't provide a native method for deepClone of Object (not that I'm aware of at least). If it did, the map could contain a clone of the MethodArg and problem solved. This becomes a challenge when a Method A would be called with Object State 1, then Object State transitioned to 2 and Method B called. In this case, A would not verify because of the above situation.

Solutions that I can think of: 1) Don't ever modify object after calling a method that is being tracked - Not really viable 2) Clone the method arg prior to calling method recorder - This becomes a challenge with the exception of List which provides deepClone. For other classes, a clone method would need to be written. If there was a "clone", method param could be cloned prior to adding to List passed to method recorder. 3) Instead of using Map<Object, Integer>, approach method arg detection in a different way

Does anyone else see this as an issue? Thoughts?

Update - One other area that this could present a problem is with Dates and DateTimes. For example, if a method uses DateTime.now, to build an object to pass in to verify with exactly the same time would require using a static member (e.g. Utils.CurrentDateTime) that gets set to DateTime.now instead of just liberally calling DateTime.now when the current DateTime is needed. Using a static member such as this that both the test method and service methods call would ensure the datetime is always the same but this does raise another concern regarding how methods are recorded/tracked/verified.

dfruddffdc commented 8 years ago

I teased about matcher support in the other issue you raised (Add support for ANY parameter #7 ).

I'm planning on implementing a RefEq matcher, so that we match arguments using === instead of ==, which should get around the issue.

There will also be a handful of helpers for matching sobjects: sObjectOfType(Schema.SObjectType objectType) sObjectWithId(Id toMatch) sObjectWithName(String toMatch) sObjectWith(Map<Schema.SObjectField, Object> requiredFieldValues)

And if none of those float your boat, you will be able to implement your own custom matcher. Watch this space :)

jondavis9898 commented 8 years ago

Again, awesome, thank you! Would be slick to have some type of Custom Matcher (e.g. callback support) for non-standard situations. Looking forward to it!