Issue #196 - Add support for specifying CalculationMode in developer API

[jondavis9898]

This is not the original desired result of the issue which would be to completely separate the "When" (calculationmode) from the "What" (rollup definition). After reviewing the code, a change of that nature would be fairly invasive and honestly and unfortunately, take more time than I have available right now.

As a compromise, I've exposed a public method that allows the modes to be specified with the previously existing global overload maintaining backwards compat and only executing Developer mode rollups. Since CalculationMode is a public enum in a public class, I was unable to make the new rollup overload global so calling this method does require that the code be installed and not the package. An unfortunate side-effect.

I think the end goal here should be to separate the "when" and "what" concerns but hopefully this is acceptable approach for the time being.

[afawcett]

Thanks for this! Curious, what was your use case / motivation for this?

[jondavis9898]

Hi Andrew -

I just updated the pull request per your guidance in #209.

The short version of the use case is that we are processing large quantities of records in our API transactions (e.g. more than 200 of a single SObject type) and this results in validations on SObjects failing in certain cases (e.g. SUM(Child.Qty) == Parent.TotalQty) because all records are not yet complete. Part of the solution to solve for this involves deferring the rollups from occurring until all DML operations are complete. Doing this provides two benefits:

1. Validation code in parent SObjects won't be executed until all DML is complete and all records visible/accessible
2. Reduces the overall number of Queries, DML Statements & DML records inside of an Execution Context to lower the risk of hitting governors.

More details on the background of the use case can be found at http://salesforce.stackexchange.com/questions/81350/validate-data-as-unit-of-work/81712#81712. (See the post from me at the bottom from 6/29 @ 22:45Z). Coupled with the changes proposed in apex-enterprise-patterns/fflib-apex-common#62, I'm able to defer all rollups from firing within the trigger context and instead keep track of all SObjects involved in the Execution Context and then force rollups to occur after all DML is complete.

The flip side to this is that I still need rollups to fire in real-time within trigger context when the operation is not coming from an API interaction (e.g. standard page layout). This is ultimately the goal of the original issue I posted (making calc mode a multi-picklist). The solution in this PR is really a "compromise" to what I'd proposed the real enhancement be. Unfortunately I'd want to come up to speed more on DLRS library to evaluate the potential impacts before making that change and would need more time to complete.

Assuming you feel that making calc mode a multi-picklist makes sense, I'll try to work towards making that change in the near future (or if someone else has the time :)).

[afawcett]

I'm hoping to keep Realtime mode focused on the dynamically deployed triggers, e.g. the triggerHandler entry point. The Developer mode on the DLRS definition is what i really intended for this kind of use case. I think whats driven you here and your other PR is that really the dlrs.RollupService.rollup method didn't do what you want. Would focusing on adding new dlrs.RollupService.rollup method that works exactly like trigerHandler work for you here?

[jondavis9898]

If I'm understanding where you are going with having a dlrs.RollupService.rollup method that works exactly like triggerHandler I think you're saying that it would do the update itself rather than return the result for the caller to be updated and also do things like handling different SObject types provided in the parameter list? If I'm misunderstanding let me know.

Assuming that's the case, in my scenario it unfortunately wouldn't provide what I need. In short, I need to be able to run a rollup in both "Realtime" and "Developer" mode. Here's an example:

A) User goes to a standard layout to update an Account. When they click save the rollup would fire immediately using trigger context (Realtime)
B) User goes to our custom VF page. When they click "save", our API is invoked that does several things one of which is to update the Account. The API marks a static variable to indicate a "transaction is occurring" so the SObject is "tracked" and execution of the rollup is deferred. In the new "onDMLFinished" of fflib apex common, all "tracked" SObjects have their rollups processed. This would typically be a "developer" mode rollup but it's the same rollup as the one in "A". Essentially, I need a single rollup to be defined as both "Developer" and "Realtime" so that it can be executed in either trigger context or via API. Since Calculation mode is a picklist, my solution was to be able to define the rollup as realtime to support "A" and then expose an API to effectively "force" a realtime rollup to recalculate for "B".

Hope this makes sense, sorry if I'm misunderstanding what you were thinking with dlrs.RollupService.rollup.

[jondavis9898]

Just read #165 per your comment in #215, I think I see where you are going now - sorry for delayed reaction ;)

Here's my current understanding and how it would apply to the scenario described in my previous post. Add an API that behaves like triggerHandler that would handle multiple SObject types, detect changes from old/new and perform the actual update. Essentially it's the same as triggerHandler except it gets its data from parameters instead of Trigger context. For my scenario, all my rollups would be marked Developer and instead of calling triggerHandler in my trigger I would call the new developer API in both scenario "A" and "B". Correct?

If so, I think this would work well and a great solution to what I'm after. The only other thing I would need is to make sure that the new developer API can "force" an update to occur. In other words, skip any "change detection" and just always force the rollup to occur. The reason for this is that within a "transaction" an SObject can be updated multiple times and outside of a trigger context, old/new isn't viable to rely on. For this reason, the new developer api could have two flavors one that just takes the SObject list and one that takes SOBject lists for both old/new.

Now that I think about it more, I could actually further optimize my side if this was exposed to take advantage of change tracking by tracking the original "old" from trigger. Sidebar thought, just thinking through how I'd approach if this was available.

Let me know if I'm understanding what you were driving at correctly and your thoughts. Thanks!

[jondavis9898]

I put together a very rough skeleton of what I think you're thinking. This by no means is intended to be "PR" ready, just a baseline to help the discussion along. See RollupService.cls at https://github.com/jondavis9898/declarative-lookup-rollup-summaries/blob/3e99e897dbe75b69f1b12ac323a0a0c90b84e579/rolluptool/src/classes/RollupService.cls.

[afawcett]

Yes if its this one...

    // existingRecords should be null if not in an isUpdate scenario
    // when forceupdate is false, if existingRecords is not null, change detection will occur
    // if forceupdate is true OR existingRecords is null, no change detection occurs and rollups are always processed
    global static void rollup(List<SObject> childRecords, Map<Id, SObject> existingRecords, Boolean forceUpdate)
    {
        handleRollups(childRecords, existingRecords, forceUpdate, new List<RollupSummaries.CalculationMode> { RollupSummaries.CalculationMode.Developer, RollupSummaries.CalculationMode.Scheduled });
    }

Though i would pass on CalculationMode.Developer to it, as per the current rollup global static List rollup(List childRecords).

[jondavis9898]

Hi Andrew - Yes, the api that you referenced is the new one that I was thinking. Not sure I follow what you mean by "pass on CalculationMode.Developer" to it? Do you mean have it accept CalculationMode as a parameter or only specifiy Developer when it calls handleRollups or something else?

If the former, I can't add CalculationMode to the method signature without changing RollupSummaries class becuase CalculationMode is public not global. If the later, is there a reason that you don't think Scheduled should be included? Or if neither of those and you meant something totally different, let me know :)

The full skeleton is in commit jondavis9898@d71861a. In there I mocked up the following changes:

1. Add new API (the one you referenced). New API calls new private handleRollups
2. Refactored existing triggerHandler to call handleRollups

Essentially, calling triggerHandler or new developer API would result in exactly the same functionality, the only difference would be "where" it gets childRecords & existingRecords from.

Let me know your thoughts and if you think there is anything inside of handleRollups that might cause a regression when invoked via Developer API directly. I'm still coming up to speed on the library so don't want to overlook anything.

Thanks!

[afawcett]

Yes this is pretty close, sorry ment to say 'Pass only CalculationMode.Developer to it', so yes i ment 'only specifiy Developer when it calls handleRollups'.

[jondavis9898]

Thanks for clarifying Andrew. Is there a reason why you don't think Scheduled should be included?

In the declaritive case, Realtime and Scheduled are included. If I'm understanding your original intent based on my proposed addition, it's to be able to invoke identical functionality but in a non-declarative manner (e.g. not have to deploy the trigger that is required in Realtime mode).

Along those lines, it's essentially just moving the call to triggerHandler in to another area in the code (e.g. inside of an SObjectDomain class for example). In this case, I would think including Scheduled would be preferred so that the net result is exactly the same as using triggerHandler via declaritive mode. Very possible I'm not fully understanding the nuances of Scheduled.

If you could share why you think Scheduled should not be included I'd appreciate it. Thanks!

[afawcett]

I'm basically wanting to keep the rollups defined by admins separate from those defined by developers to be used by the API. This way things don't end up getting processed twice if a developer ends up calling the API in the same execution context as one of the standard DLRS triggers on a child object. This essentially means Developer mode is always realtime, the thinking is if a developer thought they needed to do the query in batch they would do so in their batch context.

[jondavis9898]

Ah, gotcha, that makes sense, thanks for the clarification.

Based on your familiarity, can you see a use case where via the Developer API, there would be a need to have an async/scheduled solution (e.g. avoid governor limits, etc.)? If no current use case, possibly we just cross that bridge if/when it ever comes up?

[afawcett]

Yeah, probably be another API to push a schedule item.

[jondavis9898]

Closing this as PR #221 replaces and targets approach discussed.