apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
911 stars 517 forks source link

UnitOfWork cannot be subclassed and constructed within the UnitOfWorkFactory #377

Closed rob-baillie-ortoo closed 2 years ago

rob-baillie-ortoo commented 3 years ago

Scenario:

Problem:

Desired solution:

Suggested core changes:

ImJohnMDaniel commented 3 years ago

G'day @rob-baillie-ortoo. Thanks for reaching out.

Can you elaborate more on the scenario which would require the UOW to be extended??

rob-baillie-ortoo commented 3 years ago

In our situation, in order to be able to interrogate the UOW and find the number of DML statements / DML rows that are backed up in the transaction. This is so we can pro-actively manage adherence to limits.

I'm sure there are others...

When I look at the fflib_SObjectUnitOfWork it is currently defined as virtual, as are the methods - and the member variables are protected, rather than private - it suggests that the class was built to be extended - it's just the factory that the wasn't built to allow it.

rob-baillie-ortoo commented 3 years ago

Updated original solution to remove the introduction of fflib_ISObjectUnitOfWork, as this already exists.

ImJohnMDaniel commented 3 years ago

So, the UOW was designed to allow you to replace the default fflib_SObjectUnitOfWork.SimpleDML with a custom class of your own that implements fflib_SObjectUnitOfWork.IDML. I suspect that might be the functionality that you are looking for. If not, please let me know.

rob-baillie-ortoo commented 3 years ago

Thanks, but I don't think it does. I'm not looking to perform actions when a commit occurs, I'm looking to prevent going over the limits before the work is registered.

E.g. I may want to add the methods public Integer getNumberOfPendingDmlRows() This method works out how many DML rows would be sent when commitWork is issued.

I can then use that to check against limits to ensure that I don't overload the UOW before it happens, and then defer that work.

I guess with the IDML I could push any 'over the limit' records into another transaction, but I'm not that that would be universal.

Essentially, I want to add to the interface of the UOW so I can get more information out.

stohn777 commented 2 years ago

Hi @rob-baillie-ortoo.
I have an idea of what you're asking but not quite sure I see the same thing that you may be envisioning. Without expending numerous hours, please provide us with an example of what you'd like to do, as a more technical conversation starter. At this point, you have an idea in your head while we're guessing what that might be.

Thanks!

ImJohnMDaniel commented 2 years ago

@rob-baillie-ortoo, I might be missing something here, but doing custom actions in the commit process is exactly what passing a new IDML will allow to be done. Essentially, that is what the purpose of the IDML was designed for. Implementing the IDML interface forces the creation of the following methods:

void dmlInsert(List<SObject> objList)
void dmlUpdate(List<SObject> objList)
void dmlDelete(List<SObject> objList)
void eventPublish(List<SObject> objList)
void emptyRecycleBin(List<SObject> objList)

Once those methods are defined, you are free to add whatever custom logic you wish to those methods. Personally, I use this approach sometimes to create an AtomicDML class which uses the database.insert(List<SObject>, Boolean); method (and similar methods) instead of the SimpleDML's usage of the standard insert List<Sobject> method. The approach would certainly address your use case.

Please let me know if I am missing anything.

rob-baillie-ortoo commented 2 years ago

@ImJohnMDaniel - at this point it's too late. When you're issuing the the DML the records have all been registered and it would be very hard to pick apart what we can commit and what we can't in order to avoid breaking the limits. It's not as simple as just saying, well, I'll push the first x records and leave the rest.

@stohn777 - one option would be something along the lines of:

public inherited sharing class extended_SobjectUnitOfWork extends fflib_SObjectUnitOfWork
{
    public Integer getNumberOfPendingDmlRows()
    {
        return getCountOfNewSobjectRows()
            +  getCountOfDirtySobjectRows()
            +  getCountOfDeletedSobjectRows()
            +  getCountOfEmptyRecyleBinRows()
            +  getCountOfPublishBeforeRows()
            +  getCountOfPublishAfterSuccessRows();
    }
    // ...
}

Which is then used by a service:

public with sharing class LimitsServiceImpl implements ILimitsService
{
    public Integer getAvailableDmlRowsHeadroom( extended_SobjectUnitOfWork uow  ) {

        Integer futureDmlRows  = uow.getNumberOfPendingDmlRows();
        Integer currentDmlRows = Limits.getDmlRows();

        return ( currentDmlRows + futureDmlRows );
    }
}

In particular circumstances, the system proper would then check the headroom prior to performing work that could potentially push over the limits.

Whilst scheduled processes and suchlike can help with improving the limits management, the configurable nature of the system means that we can never be sure that an individual transaction will not result in limits being broken. We can manage the effect if we can spot it in advance and exit prior to the limits violation, but once we hit the limit we're doomed.

scottmcclung commented 2 years ago

@rob-baillie-ortoo Could you extend the Application class and override the UnitOfWorkFactory.newInstance() methods to return the extended version of the uow class instead of the standard? Something like this?

public class MyApplication extends fflib_Application
{
    public class UnitOfWorkFactory extends fflib_Application.UnitOfWorkFactory
    {
        public override fflib_ISObjectUnitOfWork newInstance()
        {
            if(m_mockUow!=null)
                return m_mockUow;
            return new Extended_SobjectUnitOfWork(m_objectTypes);
        }
    }
}

In my use case, I would like to extend UnitOfWorkFactory to construct all new instances of uow with a custom IDML implementation like the following

public class MyApplication extends fflib_Application
{
    public class UnitOfWorkFactory extends fflib_Application.UnitOfWorkFactory
    {
        public override fflib_ISObjectUnitOfWork newInstance()
        {
            if(m_mockUow!=null)
                return m_mockUow;
            return new fflib_SObjectUnitOfWork(m_objectTypes, new MyDml());
        }
    }
}

@ImJohnMDaniel @stohn777 While the fflib_Application.UnitOfWorkFactory and its newInstance() methods are virtual it seems to depart from the other application factory classes by specifying m_objectTypes and m_mockUow as private instead of protected. As a result the above examples won't compile. Any reason that we shouldn't update those to be protected?

scottmcclung commented 2 years ago

Just realized there's already a PR to address this. #352

daveespo commented 2 years ago

It sounds like #352 provided a path forward here so I'm closing this issue; if you don't agree, please reopen