apex-enterprise-patterns / fflib-apex-common

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

Creating a new Selector class for Salesforce Files? #326

Closed PseudoDarwinist closed 1 year ago

PseudoDarwinist commented 3 years ago

I am not sure if this is the the right platform to ask but I need help. I have converted a big payload to Salesforce files and then I have created a batch to convert it back to String to send it as a request to external system. Everything complex works fine but i have a query regarding a silly thing which is in my batch class i have performed a query to get the right contentDocument from my object

for(EventLog__c so : (List<EventLog__c>) sObjScope){
            ContentDocumentLink cdlList = [SELECT ContentDocumentId,
                                            ContentDocument.LatestPublishedVersion.VersionData,
                                            LinkedEntityId  
                                            FROM ContentDocumentLink 
                                            WHERE LinkedEntityId in ( 
                                                SELECT Id FROM EventLog__c Where Id =: so.Id) 
                                            AND LinkedEntity.Type='EventLog__c'];
            Blob FileBody = cdlList.ContentDocument.LatestPublishedVersion.VersionData;
            String Attachment = FileBody.toString()

But as in design principle ,we need to segregate the queries to Selector layer? Do i need to create a selector layer for Salesforce Files? or querying it directly in batch class is ok? And yes for my batch class which queries for scoped EventLog__c, i have created a selector class.

wimvelzeboer commented 3 years ago

Hi @PseudoDarwinist

One thing I immediately notice is that you are using a query inside an iteration, therefore you have a high risk of hitting limits. But to answer your question, within the Apex Enterprise Patterns you should always separate queries in a separate selector class. Otherwise you are unable to mock the queries and make true unit-test.

Looking at your code snippet I would translate that to the AEP in the following way;

// Contain the EventLog__c records inside their domain
IEventLogs eventLogs = EventLogs.newInstance(sObjScope);

// Use a getter method on the EventLogs domain to retrieve the record ids
Set<Id> eventLogIds = eventLogs.getRecordIds();

// Use the selector method of ContentDocumentLink
List<ContentDocumentLink> contentDocumentLinkRecords =
    ContentDocumentLinksSelector.selectByLinkedEntityId(eventLogIds);

// Translate the query result into a domain
IContentDocumentLinks contentDocumentLinks = ContentDocumentLinks.newInstance(contentDocumentLinkRecords);

// Use a getter to retrieve the required data
Map<Id, String> bodyAsStringByLinkedEntityId =
      contentDocumentLinks.getBodyAsStringByLinkedEntityId();    

Remember, always work in bulk.

And for the location of this logic, I would not put this in the batch class. The purpose of a batch class is that we are able to perform business logic in batches. And the high level business logic should reside on a service layer. So, the batch should not perform the business logic, but should try to call a service method as soon as possible.

PseudoDarwinist commented 3 years ago

Thank you so much @wimvelzeboer .I am a naive and a beginner wrt AEPs and Apex commons.But i will leave no stone unturned to understand it completely.I appreciate this community so much.Makes me happy that our org adopted this framework.Let me parse your response and i will come back with a query.

PseudoDarwinist commented 3 years ago

@wimvelzeboer Sorry I can take the code and make it work but I want to understand it. Here is what I understand from above:

  1. We will create two Domain classes(EventLogs,ContentDocument) and two selector Classes.
  2. A getter method(getRecordIds()) in event log domain which will call event log selector to retrieve the records.
  3. ContentDocumentLink selector will query the related files on eventlogs(as blobs).
  4. A getter method(getBodyAsStringByLinkedEntityId()) in ContentDocumentLink Domain will convert the blob into a String, which I could deserialize and send to external system.

Let me know if i am correct in my understanding?Are you suggesting something else?

As for Service layer, I am already calling it in the batch class as below

  String Attachment = FileBody.toString();
           ...cInfo = JSON.deserialize(Attachment)....

            XXXService.processEvent(eventType, cInfo);
wimvelzeboer commented 3 years ago

@PseudoDarwinist Yeah, that is the process.

You batchable will look something like:

public with sharing class MyBatchableClass implements Database.Batchable<sObject>
{
    public Database.QueryLocator start(Database.BatchableContext batchContext)
    {
        return EventLogsSelector.querylocatorBySomeCondition();
    }
    public void execute(Database.BatchableContext batchContext, List<EventLog__c> scope)
    {
       try
       {
           EventLogsService.invokeSomeBusinessLogic(scope);
       } 
       catch (Exception e)
       {
           // some error logging process goes here.
       }
    }
}

The service method will contain that high level business logic from the example i gave:

public void invokeSomeBusinessLogic(List<EventLog__c> records)
{
    IEventLogs eventLogs = EventLogs.newInstance(sObjScope);

    Set<Id> eventLogIds = eventLogs.getRecordIds();

    List<ContentDocumentLink> contentDocumentLinkRecords =
            ContentDocumentLinksSelector.selectByLinkedEntityId(eventLogIds);

    IContentDocumentLinks contentDocumentLinks = 
            ContentDocumentLinks.newInstance(contentDocumentLinkRecords);

    Map<Id, String> bodyAsStringByLinkedEntityId =
            contentDocumentLinks.getBodyAsStringByLinkedEntityId();

    ExternalService.send(bodyAsStringByLinkedEntityId);  // Handles the translating (JSON stuff) and handshake with external system.
}
PseudoDarwinist commented 3 years ago

Thanks @wimvelzeboer But creating an entire Service(facade,Interface and Implementation) for just eventLogs ,isn't it an overreach?Shouldn't Service layer aggregate logic across multiple Sobjects?Also as i said,i already have a Service layer which I am calling in the existing Batch class..Can I not import the logic to that same service class? Sorry, Just out of curiosity, I was able to accomplish the requirement with few lines of code. What justifies the level of complexity that I have to do now to accomplish the same thing with Apex commons?

daveespo commented 3 years ago

I think William was just trying to describe that the business method to retrieve and convert the data should be in a Service. It doesn't need to be its own EventLogService (i.e. it doesn't need to be named to match an SObject)

It is true that most Services are orchestrations against multiple Domains (SObjects) so they are named using the business processes that they encapsulate

wimvelzeboer commented 3 years ago

@PseudoDarwinist You are completely right, if you are never going to change the code again. The goal should not be to write small pieces of code and deliver them fast. The developer's goal (in my opinion) should be to deliver flexible source code that is easy to maintain, re-useable and easy to read.

If you separate the code into multiple methods, which are just doing a single tiny thing, then chances are high that you will be able to reuse them. At start it might look a bit of an overhead and it will slow you down as you have to write more lines, but over time you will see an increase in delivery as you start to re-use more. By putting logic in its own properly named service class, it will be much easier to find. Having a standard naming convention is also key in increasing code-reuse

What is your client suddenly want to have a button on a pagelayout, for manually resending the data to that external system (e.g. in case of an error). If you had developed all your code in the batchClass, you then either have to duplicate it or have a hard time to refactor and extract the logic. If the main logic is already in the service class, then it is just a matter or calling it from a different entry point.

PseudoDarwinist commented 3 years ago

You had me at hello !!Thank you for taking your precious time out and explaining it to me @wimvelzeboer @daveespo . I will try to implement everything according to the framework and might come back for ironing out small issues ,if any. Once again appreciate all the help.

PseudoDarwinist commented 3 years ago

@wimvelzeboer Can you please help me with the Service test class.I am sorry if it is too naive but I am trying my best to wrap my head around the pattern.

@isTest
    static void testgetFilesforEventLogs(){

        //Set up mocks 
        fflib_ApexMocks mocks = new fflib_ApexMocks();
        IRR_DOM_IContentDocumentLinks domainMock = (IRR_DOM_IContentDocumentLinks) mocks.mock(IRR_DOM_IContentDocumentLinks.class);
        IRR_SEL_IContentDocumentLinksSelector selectorMock = (IRR_SEL_IContentDocumentLinksSelector) mocks.mock(
            IRR_SEL_ContentDocumentLinksSelector.class);

        //Given
        mocks.startStubbing();
        List<IRR_EventLog__c> records = new List<IRR_EventLog__c>{
        new IRR_EventLog__c(
            id = fflib_IDGenerator.generate(IRR_EventLog__c.SObjectType)
        )};
        List<ContentDocumentLink> testContentDocument = new List<ContentDocumentLink>{
            new ContentDocumentLink(
                id = fflib_IDGenerator.generate(ContentDocumentLink.SObjectType)

        )};
        Map<Id,String> bodyAsStringByLinkedEntityId = new Map<Id,String>();

        Set<Id> IdSetEventLogs = new Map<Id,IRR_EventLog__c>(records).keySet();
        mocks.when(domainMock.sObjectType()).thenReturn(ContentDocumentLink.SObjectType);
        mocks.when(selectorMock.sObjectType()).thenReturn(ContentDocumentLink.SObjectType);
        mocks.when(selectorMock.selectByLinkedEntityId(IdSetEventLogs))
            .thenReturn(new List<ContentDocumentLink>{new ContentDocumentLink()});
        mocks.when(domainMock.getBodyAsStringByLinkedEntityId()).thenReturn(bodyAsStringByLinkedEntityId);
        mocks.stopStubbing();
        IRR_Application.Domain.setMock(domainMock);
        IRR_Application.Selector.setMock(selectorMock);

        // When
        IRR_SVC_LogsService.getFilesforEventLogs(records);

error that I

Screenshot 2021-04-13 at 4 00 24 PM

am receiving is in attached screenshot.

wimvelzeboer commented 3 years ago

Hi @PseudoDarwinist Can check if the IRR_SEL_IContentDocumentLinksSelector interface is extended from fflib_ISObjectSelector?

PseudoDarwinist commented 3 years ago

Hi @wimvelzeboer .Yes it does:

public interface IRR_SEL_IContentDocumentLinksSelector extends fflib_ISObjectSelector {

    List<ContentDocumentLink> selectByLinkedEntityId (Set<Id> IdSet);
}
PseudoDarwinist commented 3 years ago

@wimvelzeboer Also,I have another question as to how we will pass records to ContentDocumentLink Domain layer so that it iterates over them , and converts it from blob to String.

mocks.when(domainMock.sObjectType()).thenReturn(ContentDocumentLink.SObjectType);
        mocks.when(selectorMock.sObjectType()).thenReturn(ContentDocumentLink.SObjectType);
        mocks.when(selectorMock.selectByLinkedEntityId(IdSetEventLogs))
            .thenReturn(new List<ContentDocumentLink>{new ContentDocumentLink()});
        mocks.when(domainMock.getBodyAsStringByLinkedEntityId()).thenReturn(bodyAsStringByLinkedEntityId);
        mocks.stopStubbing();