Is this test for callout wrong? How to make a bulk?











up vote
6
down vote

favorite
1












I wrote mock and test for callout, but I don't think it's right. I think I check wrong things with asserts and maybe call the same methods a few times, while have another ways to get 100%. Have I?



I also still don't know how to make a bulk test for my batch. Thanks for any help.



Callout:



public class CallThePepperoni {
public List<Pepperoni__c> pepperoniList;
public HttpResponse responseService;

public static HttpRequest createRequestForToken(){
HttpRequest ourRequest = new HttpRequest();
ourRequest.setBody(myTokenString);
ourRequest.setMethod('GET');
ourRequest.setEndpoint('https://myorg.salesforce.com/services/oauth2/token');
return ourRequest;
}

public static HttpRequest createRequestForService(String token){
HttpRequest finalRequest = new HttpRequest();
finalRequest.setHeader('Authorization','Bearer ' + token);
finalRequest.setHeader('Content-Type','application/json');
finalRequest.setHeader('accept','application/json');
finalRequest.setMethod('GET');
finalRequest.setEndpoint('https://myorg.salesforce.com/services/apexrest/endpoint');
return finalRequest;
}

public class OAuth2{
public String TOKEN{get;set;}
}

public List<Pepperoni> getCalloutResponseContents() {
Http ourHttp = new Http();
HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
HttpResponse responseToken = ourHttp.send(requestForToken);
OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

if(objAuthenticationInfo.TOKEN != null) {
HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
responseService = ourHttp.send(requestForService);
pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
for(Pepperoni pep : pepperoniList) {
pep.Id = null;
}
return pepperoniList;
}
}

public List<ContentVersion> getContentVersionList(Id pepId){
List<ContentVersion> contentVersionList = [SELECT Id, ContentDocumentId FROM ContentVersion WHERE FirstPublishLocationId =: pepId];
return contentVersionList;
}

public void insertCurrentContentVersion(){
for(Pepperoni temporaryP : pepperoniList) {
if(temporaryP.VersionData__c != null) {
deletePreviousFiles(temporaryP.Id);

ContentVersion currentContentVersion = new ContentVersion(
Title = temporaryP.Title__c,
PathOnClient = temporaryP.PathOnClient__c,
VersionData = EncodingUtil.base64Decode(temporaryP.VersionData__c),
FirstPublishLocationId = temporaryP.Id
);
insert currentContentVersion;
temporaryCandidate.VersionData__c = null;
update temporaryP;
}
}
}

public void deletePreviousFiles(Id pepId) {
List<ContentVersion> contentVersionList = getContentVersionList(pepId);
List<ContentDocument> contentDocumentList = new List <ContentDocument>();

for(ContentVersion temporaryContentVersion : contentVersionList){
ContentDocument thisContentDocument = [SELECT Id FROM ContentDocument WHERE Id =: temporaryContentVersion.ContentDocumentId];
contentDocumentList.add(thisContentDocument);
}
delete contentDocumentList;
}
}


Batch:



global class BatchPep implements Database.Batchable<Integer>, Database.AllowsCallouts {
CallThePepperoni callThePepperoni = new CallThePepperoni();

global Integer operationsQuantity = 1;
global Iterable<Integer> start(Database.BatchableContext BC){
List<Integer> scope = new List<Integer>();
for(integer i = 1; i <= operationsQuantity; i++){
scope.add(i);
}
return scope;
}

global void execute(Database.BatchableContext BC, List<Integer> scope){
List<Pepperoni__c> mainList = new List<Pepperoni__c>();
for(Integer i : scope){
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
mainList.addAll(pepList);
}
upsert mainList Email__c;
callThePepperoni.insertCurrentContentVersion();
}

global void finish(Database.BatchableContext BC){
}
}


Mock:



@isTest
global class PepMock implements HttpCalloutMock {


String token;
String externalId;

public CalloutJobAdvertisementMock(String token, String externalId) {
this.token = token;
this.externalId = externalId;
}

global HTTPResponse respond(HTTPRequest request) {
HttpResponse response = new HttpResponse();
response.setHeader('Content-Type', 'application/json');
String serialized;
if(request.getEndpoint().contains('services/oauth2/token')) {
serialized = '{"'+ token +'":"FAKE-TOKEN"}';
} else {
//Here I serialized an object from another side:
serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';
}
response.setBody(serialized);
return response;
}
}


Test:



@isTest
private class CalloutPepTest {

static CallThePepperoni callThePepperoni = new CallThePepperoni();

@isTest
static void testGetCallout() {
Pepperoni__c pep = new Pepperoni__c();
insert pep;
ContentVersion cv = new Contentversion(Title = 'TestTitle', PathOnClient = 'TestPath', VersionData = EncodingUtil.base64Decode('veryLoooongRealCode'), FirstPublishLocationId = pep.Id);
List<ContentVersion> cvListTest = new List<ContentVersion>{ cv };
insert cvListTest;
String request = someFakeTokenIsHere;
Test.setMock(HttpCalloutMock.class, new CalloutJobAdvertisementMock(200, 'ACCESS_TOKEN', ''));

Test.startTest();
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
HttpResponse response = callThePepperoni.responseService;
List<ContentVersion> cvList = callThePepperoni.getContentVersionList(pep.Id);
callThePepperoni.deletePreviousFiles(pep.Id);
//Is bulk-test for batch work like this? Or not?
Id batchJobId = Database.executeBatch(new BatchPep(), 100);
Test.stopTest();

System.assertEquals(response.getBody(), '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';);
System.assertEquals(pepList.size(), 1);
System.assertEquals(cvListTest.get(0).Id, cvList.get(0).Id);
}
}









share|improve this question
























  • You wanna test batch or whole code in a single method? its kinda confusing. Also, why are you getting new access_token every time, we can only make 100 callouts in a transaction. You have written code of batch BatchPep but in test class you are executing a diffrent bacth, BatchResume
    – Pranay Jaiswal
    Dec 4 at 15:00















up vote
6
down vote

favorite
1












I wrote mock and test for callout, but I don't think it's right. I think I check wrong things with asserts and maybe call the same methods a few times, while have another ways to get 100%. Have I?



I also still don't know how to make a bulk test for my batch. Thanks for any help.



Callout:



public class CallThePepperoni {
public List<Pepperoni__c> pepperoniList;
public HttpResponse responseService;

public static HttpRequest createRequestForToken(){
HttpRequest ourRequest = new HttpRequest();
ourRequest.setBody(myTokenString);
ourRequest.setMethod('GET');
ourRequest.setEndpoint('https://myorg.salesforce.com/services/oauth2/token');
return ourRequest;
}

public static HttpRequest createRequestForService(String token){
HttpRequest finalRequest = new HttpRequest();
finalRequest.setHeader('Authorization','Bearer ' + token);
finalRequest.setHeader('Content-Type','application/json');
finalRequest.setHeader('accept','application/json');
finalRequest.setMethod('GET');
finalRequest.setEndpoint('https://myorg.salesforce.com/services/apexrest/endpoint');
return finalRequest;
}

public class OAuth2{
public String TOKEN{get;set;}
}

public List<Pepperoni> getCalloutResponseContents() {
Http ourHttp = new Http();
HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
HttpResponse responseToken = ourHttp.send(requestForToken);
OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

if(objAuthenticationInfo.TOKEN != null) {
HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
responseService = ourHttp.send(requestForService);
pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
for(Pepperoni pep : pepperoniList) {
pep.Id = null;
}
return pepperoniList;
}
}

public List<ContentVersion> getContentVersionList(Id pepId){
List<ContentVersion> contentVersionList = [SELECT Id, ContentDocumentId FROM ContentVersion WHERE FirstPublishLocationId =: pepId];
return contentVersionList;
}

public void insertCurrentContentVersion(){
for(Pepperoni temporaryP : pepperoniList) {
if(temporaryP.VersionData__c != null) {
deletePreviousFiles(temporaryP.Id);

ContentVersion currentContentVersion = new ContentVersion(
Title = temporaryP.Title__c,
PathOnClient = temporaryP.PathOnClient__c,
VersionData = EncodingUtil.base64Decode(temporaryP.VersionData__c),
FirstPublishLocationId = temporaryP.Id
);
insert currentContentVersion;
temporaryCandidate.VersionData__c = null;
update temporaryP;
}
}
}

public void deletePreviousFiles(Id pepId) {
List<ContentVersion> contentVersionList = getContentVersionList(pepId);
List<ContentDocument> contentDocumentList = new List <ContentDocument>();

for(ContentVersion temporaryContentVersion : contentVersionList){
ContentDocument thisContentDocument = [SELECT Id FROM ContentDocument WHERE Id =: temporaryContentVersion.ContentDocumentId];
contentDocumentList.add(thisContentDocument);
}
delete contentDocumentList;
}
}


Batch:



global class BatchPep implements Database.Batchable<Integer>, Database.AllowsCallouts {
CallThePepperoni callThePepperoni = new CallThePepperoni();

global Integer operationsQuantity = 1;
global Iterable<Integer> start(Database.BatchableContext BC){
List<Integer> scope = new List<Integer>();
for(integer i = 1; i <= operationsQuantity; i++){
scope.add(i);
}
return scope;
}

global void execute(Database.BatchableContext BC, List<Integer> scope){
List<Pepperoni__c> mainList = new List<Pepperoni__c>();
for(Integer i : scope){
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
mainList.addAll(pepList);
}
upsert mainList Email__c;
callThePepperoni.insertCurrentContentVersion();
}

global void finish(Database.BatchableContext BC){
}
}


Mock:



@isTest
global class PepMock implements HttpCalloutMock {


String token;
String externalId;

public CalloutJobAdvertisementMock(String token, String externalId) {
this.token = token;
this.externalId = externalId;
}

global HTTPResponse respond(HTTPRequest request) {
HttpResponse response = new HttpResponse();
response.setHeader('Content-Type', 'application/json');
String serialized;
if(request.getEndpoint().contains('services/oauth2/token')) {
serialized = '{"'+ token +'":"FAKE-TOKEN"}';
} else {
//Here I serialized an object from another side:
serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';
}
response.setBody(serialized);
return response;
}
}


Test:



@isTest
private class CalloutPepTest {

static CallThePepperoni callThePepperoni = new CallThePepperoni();

@isTest
static void testGetCallout() {
Pepperoni__c pep = new Pepperoni__c();
insert pep;
ContentVersion cv = new Contentversion(Title = 'TestTitle', PathOnClient = 'TestPath', VersionData = EncodingUtil.base64Decode('veryLoooongRealCode'), FirstPublishLocationId = pep.Id);
List<ContentVersion> cvListTest = new List<ContentVersion>{ cv };
insert cvListTest;
String request = someFakeTokenIsHere;
Test.setMock(HttpCalloutMock.class, new CalloutJobAdvertisementMock(200, 'ACCESS_TOKEN', ''));

Test.startTest();
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
HttpResponse response = callThePepperoni.responseService;
List<ContentVersion> cvList = callThePepperoni.getContentVersionList(pep.Id);
callThePepperoni.deletePreviousFiles(pep.Id);
//Is bulk-test for batch work like this? Or not?
Id batchJobId = Database.executeBatch(new BatchPep(), 100);
Test.stopTest();

System.assertEquals(response.getBody(), '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';);
System.assertEquals(pepList.size(), 1);
System.assertEquals(cvListTest.get(0).Id, cvList.get(0).Id);
}
}









share|improve this question
























  • You wanna test batch or whole code in a single method? its kinda confusing. Also, why are you getting new access_token every time, we can only make 100 callouts in a transaction. You have written code of batch BatchPep but in test class you are executing a diffrent bacth, BatchResume
    – Pranay Jaiswal
    Dec 4 at 15:00













up vote
6
down vote

favorite
1









up vote
6
down vote

favorite
1






1





I wrote mock and test for callout, but I don't think it's right. I think I check wrong things with asserts and maybe call the same methods a few times, while have another ways to get 100%. Have I?



I also still don't know how to make a bulk test for my batch. Thanks for any help.



Callout:



public class CallThePepperoni {
public List<Pepperoni__c> pepperoniList;
public HttpResponse responseService;

public static HttpRequest createRequestForToken(){
HttpRequest ourRequest = new HttpRequest();
ourRequest.setBody(myTokenString);
ourRequest.setMethod('GET');
ourRequest.setEndpoint('https://myorg.salesforce.com/services/oauth2/token');
return ourRequest;
}

public static HttpRequest createRequestForService(String token){
HttpRequest finalRequest = new HttpRequest();
finalRequest.setHeader('Authorization','Bearer ' + token);
finalRequest.setHeader('Content-Type','application/json');
finalRequest.setHeader('accept','application/json');
finalRequest.setMethod('GET');
finalRequest.setEndpoint('https://myorg.salesforce.com/services/apexrest/endpoint');
return finalRequest;
}

public class OAuth2{
public String TOKEN{get;set;}
}

public List<Pepperoni> getCalloutResponseContents() {
Http ourHttp = new Http();
HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
HttpResponse responseToken = ourHttp.send(requestForToken);
OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

if(objAuthenticationInfo.TOKEN != null) {
HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
responseService = ourHttp.send(requestForService);
pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
for(Pepperoni pep : pepperoniList) {
pep.Id = null;
}
return pepperoniList;
}
}

public List<ContentVersion> getContentVersionList(Id pepId){
List<ContentVersion> contentVersionList = [SELECT Id, ContentDocumentId FROM ContentVersion WHERE FirstPublishLocationId =: pepId];
return contentVersionList;
}

public void insertCurrentContentVersion(){
for(Pepperoni temporaryP : pepperoniList) {
if(temporaryP.VersionData__c != null) {
deletePreviousFiles(temporaryP.Id);

ContentVersion currentContentVersion = new ContentVersion(
Title = temporaryP.Title__c,
PathOnClient = temporaryP.PathOnClient__c,
VersionData = EncodingUtil.base64Decode(temporaryP.VersionData__c),
FirstPublishLocationId = temporaryP.Id
);
insert currentContentVersion;
temporaryCandidate.VersionData__c = null;
update temporaryP;
}
}
}

public void deletePreviousFiles(Id pepId) {
List<ContentVersion> contentVersionList = getContentVersionList(pepId);
List<ContentDocument> contentDocumentList = new List <ContentDocument>();

for(ContentVersion temporaryContentVersion : contentVersionList){
ContentDocument thisContentDocument = [SELECT Id FROM ContentDocument WHERE Id =: temporaryContentVersion.ContentDocumentId];
contentDocumentList.add(thisContentDocument);
}
delete contentDocumentList;
}
}


Batch:



global class BatchPep implements Database.Batchable<Integer>, Database.AllowsCallouts {
CallThePepperoni callThePepperoni = new CallThePepperoni();

global Integer operationsQuantity = 1;
global Iterable<Integer> start(Database.BatchableContext BC){
List<Integer> scope = new List<Integer>();
for(integer i = 1; i <= operationsQuantity; i++){
scope.add(i);
}
return scope;
}

global void execute(Database.BatchableContext BC, List<Integer> scope){
List<Pepperoni__c> mainList = new List<Pepperoni__c>();
for(Integer i : scope){
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
mainList.addAll(pepList);
}
upsert mainList Email__c;
callThePepperoni.insertCurrentContentVersion();
}

global void finish(Database.BatchableContext BC){
}
}


Mock:



@isTest
global class PepMock implements HttpCalloutMock {


String token;
String externalId;

public CalloutJobAdvertisementMock(String token, String externalId) {
this.token = token;
this.externalId = externalId;
}

global HTTPResponse respond(HTTPRequest request) {
HttpResponse response = new HttpResponse();
response.setHeader('Content-Type', 'application/json');
String serialized;
if(request.getEndpoint().contains('services/oauth2/token')) {
serialized = '{"'+ token +'":"FAKE-TOKEN"}';
} else {
//Here I serialized an object from another side:
serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';
}
response.setBody(serialized);
return response;
}
}


Test:



@isTest
private class CalloutPepTest {

static CallThePepperoni callThePepperoni = new CallThePepperoni();

@isTest
static void testGetCallout() {
Pepperoni__c pep = new Pepperoni__c();
insert pep;
ContentVersion cv = new Contentversion(Title = 'TestTitle', PathOnClient = 'TestPath', VersionData = EncodingUtil.base64Decode('veryLoooongRealCode'), FirstPublishLocationId = pep.Id);
List<ContentVersion> cvListTest = new List<ContentVersion>{ cv };
insert cvListTest;
String request = someFakeTokenIsHere;
Test.setMock(HttpCalloutMock.class, new CalloutJobAdvertisementMock(200, 'ACCESS_TOKEN', ''));

Test.startTest();
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
HttpResponse response = callThePepperoni.responseService;
List<ContentVersion> cvList = callThePepperoni.getContentVersionList(pep.Id);
callThePepperoni.deletePreviousFiles(pep.Id);
//Is bulk-test for batch work like this? Or not?
Id batchJobId = Database.executeBatch(new BatchPep(), 100);
Test.stopTest();

System.assertEquals(response.getBody(), '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';);
System.assertEquals(pepList.size(), 1);
System.assertEquals(cvListTest.get(0).Id, cvList.get(0).Id);
}
}









share|improve this question















I wrote mock and test for callout, but I don't think it's right. I think I check wrong things with asserts and maybe call the same methods a few times, while have another ways to get 100%. Have I?



I also still don't know how to make a bulk test for my batch. Thanks for any help.



Callout:



public class CallThePepperoni {
public List<Pepperoni__c> pepperoniList;
public HttpResponse responseService;

public static HttpRequest createRequestForToken(){
HttpRequest ourRequest = new HttpRequest();
ourRequest.setBody(myTokenString);
ourRequest.setMethod('GET');
ourRequest.setEndpoint('https://myorg.salesforce.com/services/oauth2/token');
return ourRequest;
}

public static HttpRequest createRequestForService(String token){
HttpRequest finalRequest = new HttpRequest();
finalRequest.setHeader('Authorization','Bearer ' + token);
finalRequest.setHeader('Content-Type','application/json');
finalRequest.setHeader('accept','application/json');
finalRequest.setMethod('GET');
finalRequest.setEndpoint('https://myorg.salesforce.com/services/apexrest/endpoint');
return finalRequest;
}

public class OAuth2{
public String TOKEN{get;set;}
}

public List<Pepperoni> getCalloutResponseContents() {
Http ourHttp = new Http();
HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
HttpResponse responseToken = ourHttp.send(requestForToken);
OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

if(objAuthenticationInfo.TOKEN != null) {
HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
responseService = ourHttp.send(requestForService);
pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
for(Pepperoni pep : pepperoniList) {
pep.Id = null;
}
return pepperoniList;
}
}

public List<ContentVersion> getContentVersionList(Id pepId){
List<ContentVersion> contentVersionList = [SELECT Id, ContentDocumentId FROM ContentVersion WHERE FirstPublishLocationId =: pepId];
return contentVersionList;
}

public void insertCurrentContentVersion(){
for(Pepperoni temporaryP : pepperoniList) {
if(temporaryP.VersionData__c != null) {
deletePreviousFiles(temporaryP.Id);

ContentVersion currentContentVersion = new ContentVersion(
Title = temporaryP.Title__c,
PathOnClient = temporaryP.PathOnClient__c,
VersionData = EncodingUtil.base64Decode(temporaryP.VersionData__c),
FirstPublishLocationId = temporaryP.Id
);
insert currentContentVersion;
temporaryCandidate.VersionData__c = null;
update temporaryP;
}
}
}

public void deletePreviousFiles(Id pepId) {
List<ContentVersion> contentVersionList = getContentVersionList(pepId);
List<ContentDocument> contentDocumentList = new List <ContentDocument>();

for(ContentVersion temporaryContentVersion : contentVersionList){
ContentDocument thisContentDocument = [SELECT Id FROM ContentDocument WHERE Id =: temporaryContentVersion.ContentDocumentId];
contentDocumentList.add(thisContentDocument);
}
delete contentDocumentList;
}
}


Batch:



global class BatchPep implements Database.Batchable<Integer>, Database.AllowsCallouts {
CallThePepperoni callThePepperoni = new CallThePepperoni();

global Integer operationsQuantity = 1;
global Iterable<Integer> start(Database.BatchableContext BC){
List<Integer> scope = new List<Integer>();
for(integer i = 1; i <= operationsQuantity; i++){
scope.add(i);
}
return scope;
}

global void execute(Database.BatchableContext BC, List<Integer> scope){
List<Pepperoni__c> mainList = new List<Pepperoni__c>();
for(Integer i : scope){
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
mainList.addAll(pepList);
}
upsert mainList Email__c;
callThePepperoni.insertCurrentContentVersion();
}

global void finish(Database.BatchableContext BC){
}
}


Mock:



@isTest
global class PepMock implements HttpCalloutMock {


String token;
String externalId;

public CalloutJobAdvertisementMock(String token, String externalId) {
this.token = token;
this.externalId = externalId;
}

global HTTPResponse respond(HTTPRequest request) {
HttpResponse response = new HttpResponse();
response.setHeader('Content-Type', 'application/json');
String serialized;
if(request.getEndpoint().contains('services/oauth2/token')) {
serialized = '{"'+ token +'":"FAKE-TOKEN"}';
} else {
//Here I serialized an object from another side:
serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';
}
response.setBody(serialized);
return response;
}
}


Test:



@isTest
private class CalloutPepTest {

static CallThePepperoni callThePepperoni = new CallThePepperoni();

@isTest
static void testGetCallout() {
Pepperoni__c pep = new Pepperoni__c();
insert pep;
ContentVersion cv = new Contentversion(Title = 'TestTitle', PathOnClient = 'TestPath', VersionData = EncodingUtil.base64Decode('veryLoooongRealCode'), FirstPublishLocationId = pep.Id);
List<ContentVersion> cvListTest = new List<ContentVersion>{ cv };
insert cvListTest;
String request = someFakeTokenIsHere;
Test.setMock(HttpCalloutMock.class, new CalloutJobAdvertisementMock(200, 'ACCESS_TOKEN', ''));

Test.startTest();
List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
HttpResponse response = callThePepperoni.responseService;
List<ContentVersion> cvList = callThePepperoni.getContentVersionList(pep.Id);
callThePepperoni.deletePreviousFiles(pep.Id);
//Is bulk-test for batch work like this? Or not?
Id batchJobId = Database.executeBatch(new BatchPep(), 100);
Test.stopTest();

System.assertEquals(response.getBody(), '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';);
System.assertEquals(pepList.size(), 1);
System.assertEquals(cvListTest.get(0).Id, cvList.get(0).Id);
}
}






apex unit-test batch code-coverage callout






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Dec 4 at 15:54

























asked Dec 4 at 14:40









MichaelLev19

867




867












  • You wanna test batch or whole code in a single method? its kinda confusing. Also, why are you getting new access_token every time, we can only make 100 callouts in a transaction. You have written code of batch BatchPep but in test class you are executing a diffrent bacth, BatchResume
    – Pranay Jaiswal
    Dec 4 at 15:00


















  • You wanna test batch or whole code in a single method? its kinda confusing. Also, why are you getting new access_token every time, we can only make 100 callouts in a transaction. You have written code of batch BatchPep but in test class you are executing a diffrent bacth, BatchResume
    – Pranay Jaiswal
    Dec 4 at 15:00
















You wanna test batch or whole code in a single method? its kinda confusing. Also, why are you getting new access_token every time, we can only make 100 callouts in a transaction. You have written code of batch BatchPep but in test class you are executing a diffrent bacth, BatchResume
– Pranay Jaiswal
Dec 4 at 15:00




You wanna test batch or whole code in a single method? its kinda confusing. Also, why are you getting new access_token every time, we can only make 100 callouts in a transaction. You have written code of batch BatchPep but in test class you are executing a diffrent bacth, BatchResume
– Pranay Jaiswal
Dec 4 at 15:00










2 Answers
2






active

oldest

votes

















up vote
8
down vote



accepted










I think you are on the right track. The code you're testing here is inherently somewhat more difficult to test, because you make multiple callouts and run an unusual batch class that isn't dependent on the test data you provide in test context. However, I think the class and mock you've built is along the right track.



Bulk Testing



To perform a bulk test, what you'll need to do is work on your Mock. You construct a return value here, by simply returning canned JSON:



    serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


In order to perform a relevant bulk test, you'd want to include a small number of records in that JSON. So you'd have something like



    serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6XXXX"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, ' 
+ '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6YYYY"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, '
+ '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6ZZZZ"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


(Lines broken for clarity).



Inherent platform restrictions prevent you from actually executing more than one batch iteration in test context, so you'll have to make sure that operationsQuantity is always less than or equal to your batch size when you run your tests. You could set it higher than one, however, to validate the behavior of your loop. A counter in the mock class can track how many times it's called, and you can write assertions against that.



What you can do immediately, though, is configure your Mock to return multiple objects, validating that your code handles that correctly.



Testing Other Code Paths



You may want to create multiple Mocks so that you can test your code's response to several different situations:




  • A single record returned.

  • Multiple records returned.

  • No records returned.

  • An HTTP error returned.

  • Something nonsensical returned due to problems with the remote server.


You can write multiple unit tests, and utilize a different one of those mocks in each test case to ensure that your code handles each type of return properly. Because you're also building your Mock to be responsive to different endpoints, you might want to build your Mock class so that the unit test case itself can supply the JSON to return - that way, you don't need to build many different classes. Alternately, you could create an abstract base Mock class that knows how to handle your OAuth callout, and subclass it to add the specific behaviors you want for the different test cases.



You may also want to take a look at MultiStaticResourceCalloutMock, a standard class you can use to create a Mock that returns JSON stored in a static resource based upon the endpoint that is called. This could be useful depending on how you decide you want to structure your unit test collection and their associated Mocks.



While it's not the same use case as what you're working on, I have an example from one of my test projects that shows this kind of approach to testing callouts. Here is one example. I'm calling an unreliable and rather weird API from the transit agency SEPTA, and I test my callout class in a bunch of different ways using different mocks:




  • A good response, using a StaticResourceMock to return JSON stored in a Static Resource (this is inherently a bulk test, in a sense, because the JSON includes multiple results).

  • A response where the API returns its own error structure.

  • A response where I get an HTTP result code other than 200 OK.

  • A response where I get back something that doesn't make any sense and/or isn't valid JSON.

  • A response where an unexpected exception gets thrown at the callout.


This might be overkill, but because I'm working with an unreliable service and trying to handle every possible failure condition, I like the overkill on these tests.



Refactoring Code and Multiple Tests



One thing that's making your life a little more difficult is that you've built your callout class to make a token call and a REST call in the same method. If you factor those elements out, you can streamline things so you don't have to mock both callouts at the same time.



Starting from here:



public List<Pepperoni> getCalloutResponseContents() {
Http ourHttp = new Http();
HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
HttpResponse responseToken = ourHttp.send(requestForToken);
OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

if(objAuthenticationInfo.TOKEN != null) {
HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
responseService = ourHttp.send(requestForService);
pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
for(Pepperoni pep : pepperoniList) {
pep.Id = null;
}
return pepperoniList;
}
}


what if instead you did something like this:



public List<Pepperoni> getCalloutResponseContents(String token) {
Http ourHttp = new Http();
HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
responseService = ourHttp.send(requestForService);
pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
for(Pepperoni pep : pepperoniList) {
pep.Id = null;
}
return pepperoniList;
}


and build yourself a second method to perform the OAuth callout? That nets you a couple of things:




  • You can write separate unit tests just for the callouts, each independently, using multiple simpler mocks and not worrying about the batch structure. Because you can pass a token string directly to getCalloutResponseContents(), you can test it as a unit with a simple mock that only knows how to return one kind of response.

  • Then, you can test your batch class as a unit, using a very simple mock just to validate that the batch class makes the calls into your CallThePepperoni class that you expect it to.

  • Finally, you can have a larger integration test, where you set a "real" mock and run your batch class against it to validate end-to-end behavior.


You can refactor your tests, too. Right now, I think your test class might be trying to do a bit too much, without smaller unit tests to "support" it, so to speak, by demonstrating that its component pieces of functionality work. You can do this by writing tests just for methods like insertCurrentContentVersion(), for example. A test for that method could populate pepperoniList (with records deserialized from your stock JSON, perhaps) and then call it, making assertions to check that it works as expected all by itself.



A principle that you'll find helpful in attempting this kind of refactoring, both in the tests and in the code, is that hiding state makes things tricky. Look at insertCurrentContentVersion(), for example. It does not accept a parameter. Instead, it uses an instance variable on its class to do its work. That makes it ever so slightly more difficult to test in isolation, because you have to be able, in your test, to control the value of that instance variable.



But if instead it accepts a parameter instead, and doesn't depend on internal state, you have a much easier time controlling it in a test. You can call it and feed it whatever data you want to observe the results, and you never get caught in a bind where you have to restructure your code just to get a test in there.



Now, one can probably go too far in this direction, and we could have a fun discussion about where the line is between different imperatives in code structure, but here I think it's pretty reasonable to decouple those methods from class state and just pass parameters.



As a general rule, decomposing functionality into smaller, unitary methods makes a huge difference in your ability to isolate and test each piece - and it often also makes it easier to test the pieces in assemblage (as an integration test) too. When I'm satisfied with a unit test suite, I often find that for N lines of Apex code, I have N*2 or N*3 lines of tests. Many of those test lines are boilerplate, because you're testing many different variations of a single piece of functionality. No test should be very big - big tests are harder to understand, so their failures don't tell you very much.



While the code you're working on is inherently somewhat complex, I think some small refactoring efforts like this could be very helpful.






share|improve this answer























  • Thank you so much! About Bulk Testing. If I want to add 200 records in Mock? I need to do that manually too? Or can I do it with some loop?
    – MichaelLev19
    Dec 5 at 9:09






  • 1




    If you want to test with 200 records, I would encourage you to structure your tests so that you can use a StaticResourceMock, allowing you to construct your JSON for a 200-record batch in a Static Resource rather than in Apex.
    – David Reed
    Dec 5 at 12:06


















up vote
6
down vote













You have bulkification problems all over your code. Never put any governor consumption in a loop. Mostly, you have DML operations you need to move outside your for loops.



public void insertCurrentContentVersion(){
for(Pepperoni temporaryP : pepperoniList) {
if(...) {
deletePreviousFiles(temporaryP.Id); // <-- NO
// ...
insert currentContentVersion; // <-- NO
// ...
update temporaryP; // <-- NO
}
}
}

public void deletePreviousFiles(Id pepId) {
// ...
for(ContentVersion temporaryContentVersion : contentVersionList){
ContentDocument thisContentDocument = [SELECT ... FROM ... WHERE ...]; // <-- NO
// ...
}
delete contentDocumentList; // DML and you call this method in a loop
}


Typically to bulkify you simply instantiate a collection before your loop, populate it during your loop, and then act or query after your loop.



List<MyObject__c> records = new List<MyObject__c>();
for (...)
{
records.add(...);
}
insert records;


This pattern will work for your currentContentVersion collection. As for updating VersionData__c, you are just clearing it out so not much work necessary here (though your posted code won't compile since you don't declare temporaryCandidate.





In addition to the above, the query in your deletePreviousFiles can be removed entirely. If you have the Id already, use the Database.delete(List<Id>) method.



Simply update your logic to collect Id instead of SObject:



List<Id> recordIds = new List<Id>();
for(ContentVersion temporaryContentVersion : contentVersionList){
recordIds.add(temporaryContentVersion.ContentDocumentId);
}
Database.delete(recordIds);





share|improve this answer





















    Your Answer








    StackExchange.ready(function() {
    var channelOptions = {
    tags: "".split(" "),
    id: "459"
    };
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function() {
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled) {
    StackExchange.using("snippets", function() {
    createEditor();
    });
    }
    else {
    createEditor();
    }
    });

    function createEditor() {
    StackExchange.prepareEditor({
    heartbeatType: 'answer',
    convertImagesToLinks: false,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    imageUploader: {
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    },
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    });


    }
    });














    draft saved

    draft discarded


















    StackExchange.ready(
    function () {
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsalesforce.stackexchange.com%2fquestions%2f241373%2fis-this-test-for-callout-wrong-how-to-make-a-bulk%23new-answer', 'question_page');
    }
    );

    Post as a guest















    Required, but never shown

























    2 Answers
    2






    active

    oldest

    votes








    2 Answers
    2






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes








    up vote
    8
    down vote



    accepted










    I think you are on the right track. The code you're testing here is inherently somewhat more difficult to test, because you make multiple callouts and run an unusual batch class that isn't dependent on the test data you provide in test context. However, I think the class and mock you've built is along the right track.



    Bulk Testing



    To perform a bulk test, what you'll need to do is work on your Mock. You construct a return value here, by simply returning canned JSON:



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    In order to perform a relevant bulk test, you'd want to include a small number of records in that JSON. So you'd have something like



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6XXXX"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, ' 
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6YYYY"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, '
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6ZZZZ"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    (Lines broken for clarity).



    Inherent platform restrictions prevent you from actually executing more than one batch iteration in test context, so you'll have to make sure that operationsQuantity is always less than or equal to your batch size when you run your tests. You could set it higher than one, however, to validate the behavior of your loop. A counter in the mock class can track how many times it's called, and you can write assertions against that.



    What you can do immediately, though, is configure your Mock to return multiple objects, validating that your code handles that correctly.



    Testing Other Code Paths



    You may want to create multiple Mocks so that you can test your code's response to several different situations:




    • A single record returned.

    • Multiple records returned.

    • No records returned.

    • An HTTP error returned.

    • Something nonsensical returned due to problems with the remote server.


    You can write multiple unit tests, and utilize a different one of those mocks in each test case to ensure that your code handles each type of return properly. Because you're also building your Mock to be responsive to different endpoints, you might want to build your Mock class so that the unit test case itself can supply the JSON to return - that way, you don't need to build many different classes. Alternately, you could create an abstract base Mock class that knows how to handle your OAuth callout, and subclass it to add the specific behaviors you want for the different test cases.



    You may also want to take a look at MultiStaticResourceCalloutMock, a standard class you can use to create a Mock that returns JSON stored in a static resource based upon the endpoint that is called. This could be useful depending on how you decide you want to structure your unit test collection and their associated Mocks.



    While it's not the same use case as what you're working on, I have an example from one of my test projects that shows this kind of approach to testing callouts. Here is one example. I'm calling an unreliable and rather weird API from the transit agency SEPTA, and I test my callout class in a bunch of different ways using different mocks:




    • A good response, using a StaticResourceMock to return JSON stored in a Static Resource (this is inherently a bulk test, in a sense, because the JSON includes multiple results).

    • A response where the API returns its own error structure.

    • A response where I get an HTTP result code other than 200 OK.

    • A response where I get back something that doesn't make any sense and/or isn't valid JSON.

    • A response where an unexpected exception gets thrown at the callout.


    This might be overkill, but because I'm working with an unreliable service and trying to handle every possible failure condition, I like the overkill on these tests.



    Refactoring Code and Multiple Tests



    One thing that's making your life a little more difficult is that you've built your callout class to make a token call and a REST call in the same method. If you factor those elements out, you can streamline things so you don't have to mock both callouts at the same time.



    Starting from here:



    public List<Pepperoni> getCalloutResponseContents() {
    Http ourHttp = new Http();
    HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
    HttpResponse responseToken = ourHttp.send(requestForToken);
    OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

    if(objAuthenticationInfo.TOKEN != null) {
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }
    }


    what if instead you did something like this:



    public List<Pepperoni> getCalloutResponseContents(String token) {
    Http ourHttp = new Http();
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }


    and build yourself a second method to perform the OAuth callout? That nets you a couple of things:




    • You can write separate unit tests just for the callouts, each independently, using multiple simpler mocks and not worrying about the batch structure. Because you can pass a token string directly to getCalloutResponseContents(), you can test it as a unit with a simple mock that only knows how to return one kind of response.

    • Then, you can test your batch class as a unit, using a very simple mock just to validate that the batch class makes the calls into your CallThePepperoni class that you expect it to.

    • Finally, you can have a larger integration test, where you set a "real" mock and run your batch class against it to validate end-to-end behavior.


    You can refactor your tests, too. Right now, I think your test class might be trying to do a bit too much, without smaller unit tests to "support" it, so to speak, by demonstrating that its component pieces of functionality work. You can do this by writing tests just for methods like insertCurrentContentVersion(), for example. A test for that method could populate pepperoniList (with records deserialized from your stock JSON, perhaps) and then call it, making assertions to check that it works as expected all by itself.



    A principle that you'll find helpful in attempting this kind of refactoring, both in the tests and in the code, is that hiding state makes things tricky. Look at insertCurrentContentVersion(), for example. It does not accept a parameter. Instead, it uses an instance variable on its class to do its work. That makes it ever so slightly more difficult to test in isolation, because you have to be able, in your test, to control the value of that instance variable.



    But if instead it accepts a parameter instead, and doesn't depend on internal state, you have a much easier time controlling it in a test. You can call it and feed it whatever data you want to observe the results, and you never get caught in a bind where you have to restructure your code just to get a test in there.



    Now, one can probably go too far in this direction, and we could have a fun discussion about where the line is between different imperatives in code structure, but here I think it's pretty reasonable to decouple those methods from class state and just pass parameters.



    As a general rule, decomposing functionality into smaller, unitary methods makes a huge difference in your ability to isolate and test each piece - and it often also makes it easier to test the pieces in assemblage (as an integration test) too. When I'm satisfied with a unit test suite, I often find that for N lines of Apex code, I have N*2 or N*3 lines of tests. Many of those test lines are boilerplate, because you're testing many different variations of a single piece of functionality. No test should be very big - big tests are harder to understand, so their failures don't tell you very much.



    While the code you're working on is inherently somewhat complex, I think some small refactoring efforts like this could be very helpful.






    share|improve this answer























    • Thank you so much! About Bulk Testing. If I want to add 200 records in Mock? I need to do that manually too? Or can I do it with some loop?
      – MichaelLev19
      Dec 5 at 9:09






    • 1




      If you want to test with 200 records, I would encourage you to structure your tests so that you can use a StaticResourceMock, allowing you to construct your JSON for a 200-record batch in a Static Resource rather than in Apex.
      – David Reed
      Dec 5 at 12:06















    up vote
    8
    down vote



    accepted










    I think you are on the right track. The code you're testing here is inherently somewhat more difficult to test, because you make multiple callouts and run an unusual batch class that isn't dependent on the test data you provide in test context. However, I think the class and mock you've built is along the right track.



    Bulk Testing



    To perform a bulk test, what you'll need to do is work on your Mock. You construct a return value here, by simply returning canned JSON:



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    In order to perform a relevant bulk test, you'd want to include a small number of records in that JSON. So you'd have something like



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6XXXX"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, ' 
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6YYYY"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, '
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6ZZZZ"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    (Lines broken for clarity).



    Inherent platform restrictions prevent you from actually executing more than one batch iteration in test context, so you'll have to make sure that operationsQuantity is always less than or equal to your batch size when you run your tests. You could set it higher than one, however, to validate the behavior of your loop. A counter in the mock class can track how many times it's called, and you can write assertions against that.



    What you can do immediately, though, is configure your Mock to return multiple objects, validating that your code handles that correctly.



    Testing Other Code Paths



    You may want to create multiple Mocks so that you can test your code's response to several different situations:




    • A single record returned.

    • Multiple records returned.

    • No records returned.

    • An HTTP error returned.

    • Something nonsensical returned due to problems with the remote server.


    You can write multiple unit tests, and utilize a different one of those mocks in each test case to ensure that your code handles each type of return properly. Because you're also building your Mock to be responsive to different endpoints, you might want to build your Mock class so that the unit test case itself can supply the JSON to return - that way, you don't need to build many different classes. Alternately, you could create an abstract base Mock class that knows how to handle your OAuth callout, and subclass it to add the specific behaviors you want for the different test cases.



    You may also want to take a look at MultiStaticResourceCalloutMock, a standard class you can use to create a Mock that returns JSON stored in a static resource based upon the endpoint that is called. This could be useful depending on how you decide you want to structure your unit test collection and their associated Mocks.



    While it's not the same use case as what you're working on, I have an example from one of my test projects that shows this kind of approach to testing callouts. Here is one example. I'm calling an unreliable and rather weird API from the transit agency SEPTA, and I test my callout class in a bunch of different ways using different mocks:




    • A good response, using a StaticResourceMock to return JSON stored in a Static Resource (this is inherently a bulk test, in a sense, because the JSON includes multiple results).

    • A response where the API returns its own error structure.

    • A response where I get an HTTP result code other than 200 OK.

    • A response where I get back something that doesn't make any sense and/or isn't valid JSON.

    • A response where an unexpected exception gets thrown at the callout.


    This might be overkill, but because I'm working with an unreliable service and trying to handle every possible failure condition, I like the overkill on these tests.



    Refactoring Code and Multiple Tests



    One thing that's making your life a little more difficult is that you've built your callout class to make a token call and a REST call in the same method. If you factor those elements out, you can streamline things so you don't have to mock both callouts at the same time.



    Starting from here:



    public List<Pepperoni> getCalloutResponseContents() {
    Http ourHttp = new Http();
    HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
    HttpResponse responseToken = ourHttp.send(requestForToken);
    OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

    if(objAuthenticationInfo.TOKEN != null) {
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }
    }


    what if instead you did something like this:



    public List<Pepperoni> getCalloutResponseContents(String token) {
    Http ourHttp = new Http();
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }


    and build yourself a second method to perform the OAuth callout? That nets you a couple of things:




    • You can write separate unit tests just for the callouts, each independently, using multiple simpler mocks and not worrying about the batch structure. Because you can pass a token string directly to getCalloutResponseContents(), you can test it as a unit with a simple mock that only knows how to return one kind of response.

    • Then, you can test your batch class as a unit, using a very simple mock just to validate that the batch class makes the calls into your CallThePepperoni class that you expect it to.

    • Finally, you can have a larger integration test, where you set a "real" mock and run your batch class against it to validate end-to-end behavior.


    You can refactor your tests, too. Right now, I think your test class might be trying to do a bit too much, without smaller unit tests to "support" it, so to speak, by demonstrating that its component pieces of functionality work. You can do this by writing tests just for methods like insertCurrentContentVersion(), for example. A test for that method could populate pepperoniList (with records deserialized from your stock JSON, perhaps) and then call it, making assertions to check that it works as expected all by itself.



    A principle that you'll find helpful in attempting this kind of refactoring, both in the tests and in the code, is that hiding state makes things tricky. Look at insertCurrentContentVersion(), for example. It does not accept a parameter. Instead, it uses an instance variable on its class to do its work. That makes it ever so slightly more difficult to test in isolation, because you have to be able, in your test, to control the value of that instance variable.



    But if instead it accepts a parameter instead, and doesn't depend on internal state, you have a much easier time controlling it in a test. You can call it and feed it whatever data you want to observe the results, and you never get caught in a bind where you have to restructure your code just to get a test in there.



    Now, one can probably go too far in this direction, and we could have a fun discussion about where the line is between different imperatives in code structure, but here I think it's pretty reasonable to decouple those methods from class state and just pass parameters.



    As a general rule, decomposing functionality into smaller, unitary methods makes a huge difference in your ability to isolate and test each piece - and it often also makes it easier to test the pieces in assemblage (as an integration test) too. When I'm satisfied with a unit test suite, I often find that for N lines of Apex code, I have N*2 or N*3 lines of tests. Many of those test lines are boilerplate, because you're testing many different variations of a single piece of functionality. No test should be very big - big tests are harder to understand, so their failures don't tell you very much.



    While the code you're working on is inherently somewhat complex, I think some small refactoring efforts like this could be very helpful.






    share|improve this answer























    • Thank you so much! About Bulk Testing. If I want to add 200 records in Mock? I need to do that manually too? Or can I do it with some loop?
      – MichaelLev19
      Dec 5 at 9:09






    • 1




      If you want to test with 200 records, I would encourage you to structure your tests so that you can use a StaticResourceMock, allowing you to construct your JSON for a 200-record batch in a Static Resource rather than in Apex.
      – David Reed
      Dec 5 at 12:06













    up vote
    8
    down vote



    accepted







    up vote
    8
    down vote



    accepted






    I think you are on the right track. The code you're testing here is inherently somewhat more difficult to test, because you make multiple callouts and run an unusual batch class that isn't dependent on the test data you provide in test context. However, I think the class and mock you've built is along the right track.



    Bulk Testing



    To perform a bulk test, what you'll need to do is work on your Mock. You construct a return value here, by simply returning canned JSON:



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    In order to perform a relevant bulk test, you'd want to include a small number of records in that JSON. So you'd have something like



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6XXXX"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, ' 
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6YYYY"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, '
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6ZZZZ"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    (Lines broken for clarity).



    Inherent platform restrictions prevent you from actually executing more than one batch iteration in test context, so you'll have to make sure that operationsQuantity is always less than or equal to your batch size when you run your tests. You could set it higher than one, however, to validate the behavior of your loop. A counter in the mock class can track how many times it's called, and you can write assertions against that.



    What you can do immediately, though, is configure your Mock to return multiple objects, validating that your code handles that correctly.



    Testing Other Code Paths



    You may want to create multiple Mocks so that you can test your code's response to several different situations:




    • A single record returned.

    • Multiple records returned.

    • No records returned.

    • An HTTP error returned.

    • Something nonsensical returned due to problems with the remote server.


    You can write multiple unit tests, and utilize a different one of those mocks in each test case to ensure that your code handles each type of return properly. Because you're also building your Mock to be responsive to different endpoints, you might want to build your Mock class so that the unit test case itself can supply the JSON to return - that way, you don't need to build many different classes. Alternately, you could create an abstract base Mock class that knows how to handle your OAuth callout, and subclass it to add the specific behaviors you want for the different test cases.



    You may also want to take a look at MultiStaticResourceCalloutMock, a standard class you can use to create a Mock that returns JSON stored in a static resource based upon the endpoint that is called. This could be useful depending on how you decide you want to structure your unit test collection and their associated Mocks.



    While it's not the same use case as what you're working on, I have an example from one of my test projects that shows this kind of approach to testing callouts. Here is one example. I'm calling an unreliable and rather weird API from the transit agency SEPTA, and I test my callout class in a bunch of different ways using different mocks:




    • A good response, using a StaticResourceMock to return JSON stored in a Static Resource (this is inherently a bulk test, in a sense, because the JSON includes multiple results).

    • A response where the API returns its own error structure.

    • A response where I get an HTTP result code other than 200 OK.

    • A response where I get back something that doesn't make any sense and/or isn't valid JSON.

    • A response where an unexpected exception gets thrown at the callout.


    This might be overkill, but because I'm working with an unreliable service and trying to handle every possible failure condition, I like the overkill on these tests.



    Refactoring Code and Multiple Tests



    One thing that's making your life a little more difficult is that you've built your callout class to make a token call and a REST call in the same method. If you factor those elements out, you can streamline things so you don't have to mock both callouts at the same time.



    Starting from here:



    public List<Pepperoni> getCalloutResponseContents() {
    Http ourHttp = new Http();
    HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
    HttpResponse responseToken = ourHttp.send(requestForToken);
    OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

    if(objAuthenticationInfo.TOKEN != null) {
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }
    }


    what if instead you did something like this:



    public List<Pepperoni> getCalloutResponseContents(String token) {
    Http ourHttp = new Http();
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }


    and build yourself a second method to perform the OAuth callout? That nets you a couple of things:




    • You can write separate unit tests just for the callouts, each independently, using multiple simpler mocks and not worrying about the batch structure. Because you can pass a token string directly to getCalloutResponseContents(), you can test it as a unit with a simple mock that only knows how to return one kind of response.

    • Then, you can test your batch class as a unit, using a very simple mock just to validate that the batch class makes the calls into your CallThePepperoni class that you expect it to.

    • Finally, you can have a larger integration test, where you set a "real" mock and run your batch class against it to validate end-to-end behavior.


    You can refactor your tests, too. Right now, I think your test class might be trying to do a bit too much, without smaller unit tests to "support" it, so to speak, by demonstrating that its component pieces of functionality work. You can do this by writing tests just for methods like insertCurrentContentVersion(), for example. A test for that method could populate pepperoniList (with records deserialized from your stock JSON, perhaps) and then call it, making assertions to check that it works as expected all by itself.



    A principle that you'll find helpful in attempting this kind of refactoring, both in the tests and in the code, is that hiding state makes things tricky. Look at insertCurrentContentVersion(), for example. It does not accept a parameter. Instead, it uses an instance variable on its class to do its work. That makes it ever so slightly more difficult to test in isolation, because you have to be able, in your test, to control the value of that instance variable.



    But if instead it accepts a parameter instead, and doesn't depend on internal state, you have a much easier time controlling it in a test. You can call it and feed it whatever data you want to observe the results, and you never get caught in a bind where you have to restructure your code just to get a test in there.



    Now, one can probably go too far in this direction, and we could have a fun discussion about where the line is between different imperatives in code structure, but here I think it's pretty reasonable to decouple those methods from class state and just pass parameters.



    As a general rule, decomposing functionality into smaller, unitary methods makes a huge difference in your ability to isolate and test each piece - and it often also makes it easier to test the pieces in assemblage (as an integration test) too. When I'm satisfied with a unit test suite, I often find that for N lines of Apex code, I have N*2 or N*3 lines of tests. Many of those test lines are boilerplate, because you're testing many different variations of a single piece of functionality. No test should be very big - big tests are harder to understand, so their failures don't tell you very much.



    While the code you're working on is inherently somewhat complex, I think some small refactoring efforts like this could be very helpful.






    share|improve this answer














    I think you are on the right track. The code you're testing here is inherently somewhat more difficult to test, because you make multiple callouts and run an unusual batch class that isn't dependent on the test data you provide in test context. However, I think the class and mock you've built is along the right track.



    Bulk Testing



    To perform a bulk test, what you'll need to do is work on your Mock. You construct a return value here, by simply returning canned JSON:



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    In order to perform a relevant bulk test, you'd want to include a small number of records in that JSON. So you'd have something like



        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6XXXX"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, ' 
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6YYYY"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, '
    + '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6ZZZZ"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';


    (Lines broken for clarity).



    Inherent platform restrictions prevent you from actually executing more than one batch iteration in test context, so you'll have to make sure that operationsQuantity is always less than or equal to your batch size when you run your tests. You could set it higher than one, however, to validate the behavior of your loop. A counter in the mock class can track how many times it's called, and you can write assertions against that.



    What you can do immediately, though, is configure your Mock to return multiple objects, validating that your code handles that correctly.



    Testing Other Code Paths



    You may want to create multiple Mocks so that you can test your code's response to several different situations:




    • A single record returned.

    • Multiple records returned.

    • No records returned.

    • An HTTP error returned.

    • Something nonsensical returned due to problems with the remote server.


    You can write multiple unit tests, and utilize a different one of those mocks in each test case to ensure that your code handles each type of return properly. Because you're also building your Mock to be responsive to different endpoints, you might want to build your Mock class so that the unit test case itself can supply the JSON to return - that way, you don't need to build many different classes. Alternately, you could create an abstract base Mock class that knows how to handle your OAuth callout, and subclass it to add the specific behaviors you want for the different test cases.



    You may also want to take a look at MultiStaticResourceCalloutMock, a standard class you can use to create a Mock that returns JSON stored in a static resource based upon the endpoint that is called. This could be useful depending on how you decide you want to structure your unit test collection and their associated Mocks.



    While it's not the same use case as what you're working on, I have an example from one of my test projects that shows this kind of approach to testing callouts. Here is one example. I'm calling an unreliable and rather weird API from the transit agency SEPTA, and I test my callout class in a bunch of different ways using different mocks:




    • A good response, using a StaticResourceMock to return JSON stored in a Static Resource (this is inherently a bulk test, in a sense, because the JSON includes multiple results).

    • A response where the API returns its own error structure.

    • A response where I get an HTTP result code other than 200 OK.

    • A response where I get back something that doesn't make any sense and/or isn't valid JSON.

    • A response where an unexpected exception gets thrown at the callout.


    This might be overkill, but because I'm working with an unreliable service and trying to handle every possible failure condition, I like the overkill on these tests.



    Refactoring Code and Multiple Tests



    One thing that's making your life a little more difficult is that you've built your callout class to make a token call and a REST call in the same method. If you factor those elements out, you can streamline things so you don't have to mock both callouts at the same time.



    Starting from here:



    public List<Pepperoni> getCalloutResponseContents() {
    Http ourHttp = new Http();
    HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
    HttpResponse responseToken = ourHttp.send(requestForToken);
    OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

    if(objAuthenticationInfo.TOKEN != null) {
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }
    }


    what if instead you did something like this:



    public List<Pepperoni> getCalloutResponseContents(String token) {
    Http ourHttp = new Http();
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
    pep.Id = null;
    }
    return pepperoniList;
    }


    and build yourself a second method to perform the OAuth callout? That nets you a couple of things:




    • You can write separate unit tests just for the callouts, each independently, using multiple simpler mocks and not worrying about the batch structure. Because you can pass a token string directly to getCalloutResponseContents(), you can test it as a unit with a simple mock that only knows how to return one kind of response.

    • Then, you can test your batch class as a unit, using a very simple mock just to validate that the batch class makes the calls into your CallThePepperoni class that you expect it to.

    • Finally, you can have a larger integration test, where you set a "real" mock and run your batch class against it to validate end-to-end behavior.


    You can refactor your tests, too. Right now, I think your test class might be trying to do a bit too much, without smaller unit tests to "support" it, so to speak, by demonstrating that its component pieces of functionality work. You can do this by writing tests just for methods like insertCurrentContentVersion(), for example. A test for that method could populate pepperoniList (with records deserialized from your stock JSON, perhaps) and then call it, making assertions to check that it works as expected all by itself.



    A principle that you'll find helpful in attempting this kind of refactoring, both in the tests and in the code, is that hiding state makes things tricky. Look at insertCurrentContentVersion(), for example. It does not accept a parameter. Instead, it uses an instance variable on its class to do its work. That makes it ever so slightly more difficult to test in isolation, because you have to be able, in your test, to control the value of that instance variable.



    But if instead it accepts a parameter instead, and doesn't depend on internal state, you have a much easier time controlling it in a test. You can call it and feed it whatever data you want to observe the results, and you never get caught in a bind where you have to restructure your code just to get a test in there.



    Now, one can probably go too far in this direction, and we could have a fun discussion about where the line is between different imperatives in code structure, but here I think it's pretty reasonable to decouple those methods from class state and just pass parameters.



    As a general rule, decomposing functionality into smaller, unitary methods makes a huge difference in your ability to isolate and test each piece - and it often also makes it easier to test the pieces in assemblage (as an integration test) too. When I'm satisfied with a unit test suite, I often find that for N lines of Apex code, I have N*2 or N*3 lines of tests. Many of those test lines are boilerplate, because you're testing many different variations of a single piece of functionality. No test should be very big - big tests are harder to understand, so their failures don't tell you very much.



    While the code you're working on is inherently somewhat complex, I think some small refactoring efforts like this could be very helpful.







    share|improve this answer














    share|improve this answer



    share|improve this answer








    edited Dec 5 at 3:55

























    answered Dec 4 at 18:31









    David Reed

    28.2k61746




    28.2k61746












    • Thank you so much! About Bulk Testing. If I want to add 200 records in Mock? I need to do that manually too? Or can I do it with some loop?
      – MichaelLev19
      Dec 5 at 9:09






    • 1




      If you want to test with 200 records, I would encourage you to structure your tests so that you can use a StaticResourceMock, allowing you to construct your JSON for a 200-record batch in a Static Resource rather than in Apex.
      – David Reed
      Dec 5 at 12:06


















    • Thank you so much! About Bulk Testing. If I want to add 200 records in Mock? I need to do that manually too? Or can I do it with some loop?
      – MichaelLev19
      Dec 5 at 9:09






    • 1




      If you want to test with 200 records, I would encourage you to structure your tests so that you can use a StaticResourceMock, allowing you to construct your JSON for a 200-record batch in a Static Resource rather than in Apex.
      – David Reed
      Dec 5 at 12:06
















    Thank you so much! About Bulk Testing. If I want to add 200 records in Mock? I need to do that manually too? Or can I do it with some loop?
    – MichaelLev19
    Dec 5 at 9:09




    Thank you so much! About Bulk Testing. If I want to add 200 records in Mock? I need to do that manually too? Or can I do it with some loop?
    – MichaelLev19
    Dec 5 at 9:09




    1




    1




    If you want to test with 200 records, I would encourage you to structure your tests so that you can use a StaticResourceMock, allowing you to construct your JSON for a 200-record batch in a Static Resource rather than in Apex.
    – David Reed
    Dec 5 at 12:06




    If you want to test with 200 records, I would encourage you to structure your tests so that you can use a StaticResourceMock, allowing you to construct your JSON for a 200-record batch in a Static Resource rather than in Apex.
    – David Reed
    Dec 5 at 12:06












    up vote
    6
    down vote













    You have bulkification problems all over your code. Never put any governor consumption in a loop. Mostly, you have DML operations you need to move outside your for loops.



    public void insertCurrentContentVersion(){
    for(Pepperoni temporaryP : pepperoniList) {
    if(...) {
    deletePreviousFiles(temporaryP.Id); // <-- NO
    // ...
    insert currentContentVersion; // <-- NO
    // ...
    update temporaryP; // <-- NO
    }
    }
    }

    public void deletePreviousFiles(Id pepId) {
    // ...
    for(ContentVersion temporaryContentVersion : contentVersionList){
    ContentDocument thisContentDocument = [SELECT ... FROM ... WHERE ...]; // <-- NO
    // ...
    }
    delete contentDocumentList; // DML and you call this method in a loop
    }


    Typically to bulkify you simply instantiate a collection before your loop, populate it during your loop, and then act or query after your loop.



    List<MyObject__c> records = new List<MyObject__c>();
    for (...)
    {
    records.add(...);
    }
    insert records;


    This pattern will work for your currentContentVersion collection. As for updating VersionData__c, you are just clearing it out so not much work necessary here (though your posted code won't compile since you don't declare temporaryCandidate.





    In addition to the above, the query in your deletePreviousFiles can be removed entirely. If you have the Id already, use the Database.delete(List<Id>) method.



    Simply update your logic to collect Id instead of SObject:



    List<Id> recordIds = new List<Id>();
    for(ContentVersion temporaryContentVersion : contentVersionList){
    recordIds.add(temporaryContentVersion.ContentDocumentId);
    }
    Database.delete(recordIds);





    share|improve this answer

























      up vote
      6
      down vote













      You have bulkification problems all over your code. Never put any governor consumption in a loop. Mostly, you have DML operations you need to move outside your for loops.



      public void insertCurrentContentVersion(){
      for(Pepperoni temporaryP : pepperoniList) {
      if(...) {
      deletePreviousFiles(temporaryP.Id); // <-- NO
      // ...
      insert currentContentVersion; // <-- NO
      // ...
      update temporaryP; // <-- NO
      }
      }
      }

      public void deletePreviousFiles(Id pepId) {
      // ...
      for(ContentVersion temporaryContentVersion : contentVersionList){
      ContentDocument thisContentDocument = [SELECT ... FROM ... WHERE ...]; // <-- NO
      // ...
      }
      delete contentDocumentList; // DML and you call this method in a loop
      }


      Typically to bulkify you simply instantiate a collection before your loop, populate it during your loop, and then act or query after your loop.



      List<MyObject__c> records = new List<MyObject__c>();
      for (...)
      {
      records.add(...);
      }
      insert records;


      This pattern will work for your currentContentVersion collection. As for updating VersionData__c, you are just clearing it out so not much work necessary here (though your posted code won't compile since you don't declare temporaryCandidate.





      In addition to the above, the query in your deletePreviousFiles can be removed entirely. If you have the Id already, use the Database.delete(List<Id>) method.



      Simply update your logic to collect Id instead of SObject:



      List<Id> recordIds = new List<Id>();
      for(ContentVersion temporaryContentVersion : contentVersionList){
      recordIds.add(temporaryContentVersion.ContentDocumentId);
      }
      Database.delete(recordIds);





      share|improve this answer























        up vote
        6
        down vote










        up vote
        6
        down vote









        You have bulkification problems all over your code. Never put any governor consumption in a loop. Mostly, you have DML operations you need to move outside your for loops.



        public void insertCurrentContentVersion(){
        for(Pepperoni temporaryP : pepperoniList) {
        if(...) {
        deletePreviousFiles(temporaryP.Id); // <-- NO
        // ...
        insert currentContentVersion; // <-- NO
        // ...
        update temporaryP; // <-- NO
        }
        }
        }

        public void deletePreviousFiles(Id pepId) {
        // ...
        for(ContentVersion temporaryContentVersion : contentVersionList){
        ContentDocument thisContentDocument = [SELECT ... FROM ... WHERE ...]; // <-- NO
        // ...
        }
        delete contentDocumentList; // DML and you call this method in a loop
        }


        Typically to bulkify you simply instantiate a collection before your loop, populate it during your loop, and then act or query after your loop.



        List<MyObject__c> records = new List<MyObject__c>();
        for (...)
        {
        records.add(...);
        }
        insert records;


        This pattern will work for your currentContentVersion collection. As for updating VersionData__c, you are just clearing it out so not much work necessary here (though your posted code won't compile since you don't declare temporaryCandidate.





        In addition to the above, the query in your deletePreviousFiles can be removed entirely. If you have the Id already, use the Database.delete(List<Id>) method.



        Simply update your logic to collect Id instead of SObject:



        List<Id> recordIds = new List<Id>();
        for(ContentVersion temporaryContentVersion : contentVersionList){
        recordIds.add(temporaryContentVersion.ContentDocumentId);
        }
        Database.delete(recordIds);





        share|improve this answer












        You have bulkification problems all over your code. Never put any governor consumption in a loop. Mostly, you have DML operations you need to move outside your for loops.



        public void insertCurrentContentVersion(){
        for(Pepperoni temporaryP : pepperoniList) {
        if(...) {
        deletePreviousFiles(temporaryP.Id); // <-- NO
        // ...
        insert currentContentVersion; // <-- NO
        // ...
        update temporaryP; // <-- NO
        }
        }
        }

        public void deletePreviousFiles(Id pepId) {
        // ...
        for(ContentVersion temporaryContentVersion : contentVersionList){
        ContentDocument thisContentDocument = [SELECT ... FROM ... WHERE ...]; // <-- NO
        // ...
        }
        delete contentDocumentList; // DML and you call this method in a loop
        }


        Typically to bulkify you simply instantiate a collection before your loop, populate it during your loop, and then act or query after your loop.



        List<MyObject__c> records = new List<MyObject__c>();
        for (...)
        {
        records.add(...);
        }
        insert records;


        This pattern will work for your currentContentVersion collection. As for updating VersionData__c, you are just clearing it out so not much work necessary here (though your posted code won't compile since you don't declare temporaryCandidate.





        In addition to the above, the query in your deletePreviousFiles can be removed entirely. If you have the Id already, use the Database.delete(List<Id>) method.



        Simply update your logic to collect Id instead of SObject:



        List<Id> recordIds = new List<Id>();
        for(ContentVersion temporaryContentVersion : contentVersionList){
        recordIds.add(temporaryContentVersion.ContentDocumentId);
        }
        Database.delete(recordIds);






        share|improve this answer












        share|improve this answer



        share|improve this answer










        answered Dec 4 at 18:52









        Adrian Larson

        104k19111234




        104k19111234






























            draft saved

            draft discarded




















































            Thanks for contributing an answer to Salesforce Stack Exchange!


            • Please be sure to answer the question. Provide details and share your research!

            But avoid



            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.


            To learn more, see our tips on writing great answers.





            Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


            Please pay close attention to the following guidance:


            • Please be sure to answer the question. Provide details and share your research!

            But avoid



            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.


            To learn more, see our tips on writing great answers.




            draft saved


            draft discarded














            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsalesforce.stackexchange.com%2fquestions%2f241373%2fis-this-test-for-callout-wrong-how-to-make-a-bulk%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown





















































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown

































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown







            Popular posts from this blog

            Сан-Квентин

            Алькесар

            Josef Freinademetz