diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/SecureDml.cls b/framework/default/ortoo-core/default/classes/fflib-extension/SecureDml.cls index be0a848f148..8213fb109b2 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/SecureDml.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/SecureDml.cls @@ -289,7 +289,7 @@ public inherited sharing virtual class SecureDml extends fflib_SobjectUnitOfWork String sobjectTypeName = SobjectUtils.getSobjectName( objList[0] ); SecurityDecision securityDecision = stripInaccessible( mode, objList ); - if ( securityDecision.fieldsWereRemoved() ) + if ( securityDecision.fieldsWereRemoved( sobjectTypeName ) ) { Set removedFields = securityDecision.getRemovedFieldsFor( sobjectTypeName ); List strippedRecords = securityDecision.getRecords(); @@ -340,8 +340,6 @@ public inherited sharing virtual class SecureDml extends fflib_SobjectUnitOfWork return remainingRemovedFields; } - // We need to put the ignored, but previously stripped, fields back - List unstrippedRecords = new List(); for ( Integer i=0; i < originalRecords.size(); i++ ) { Sobject thisOriginalRecord = originalRecords[i]; @@ -356,10 +354,8 @@ public inherited sharing virtual class SecureDml extends fflib_SobjectUnitOfWork thisStrippedRecord.put( thisFieldToUnstrip, thisOriginalRecord.get( thisFieldToUnstrip ) ); } } - unstrippedRecords.add( thisStrippedRecord ); } - replaceList( strippedRecords, unstrippedRecords ); return remainingRemovedFields; } @@ -434,7 +430,7 @@ public inherited sharing virtual class SecureDml extends fflib_SobjectUnitOfWork Contract.requires( newList != null, 'replaceList called with a null newList' ); Contract.requires( originalList.size() == newList.size(), 'replaceList called with lists that are different sizes' ); - for ( Integer i; i < originalList.size(); i++ ) + for ( Integer i=0; i < originalList.size(); i++ ) { originalList[i] = newList[i]; } @@ -504,13 +500,14 @@ public inherited sharing virtual class SecureDml extends fflib_SobjectUnitOfWork } /** - * States if fields were removed from the sobjects + * States if fields were removed from the given Sobject Type * - * @return Boolean Where fields removed? + * @param String The Name of the Sobject Type to check + * @return Boolean Were fields removed */ - public Boolean fieldsWereRemoved() + public Boolean fieldsWereRemoved( String sobjectTypeName ) { - return !records.isEmpty(); + return !removedFields.isEmpty() && removedFields.containsKey( sobjectTypeName ); } /** diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/tests/SecureDmlTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/tests/SecureDmlTest.cls index 5bbe44ca896..abb0c37ae7e 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/tests/SecureDmlTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/tests/SecureDmlTest.cls @@ -1,6 +1,8 @@ @isTest private without sharing class SecureDmlTest { + private static final Integer LOTS_OF_RECORDS = 10000; + @isTest private static void dmlInsert_whenTheUserCanCreateTheRecords_willInsertTheRecords() // NOPMD: Test method name format { @@ -26,6 +28,27 @@ private without sharing class SecureDmlTest System.assertEquals( 'Account2', createdAccounts[1].Name, 'dmlInsert, when the user can create the records, will insert the records, setting the fields on the records that are created (1)' ); } + @isTest + private static void dmlInsert_whenAskedToCreateALotOfRecords_willInsertTheRecords() // NOPMD: Test method name format + { + List accounts = new List(); + for ( Integer i=0; i < LOTS_OF_RECORDS; i++ ) + { + accounts.add( new Account( Name = 'Account' + i ) ); + } + + Test.startTest(); + + SecureDml dml = new TestableSecureDml(); + dml.dmlInsert( accounts ); + + Test.stopTest(); + + List createdAccounts = getAccountsInserted(); + + System.assertEquals( LOTS_OF_RECORDS, createdAccounts.size(), 'dmlInsert, when the user can create the records, will insert the records without blowing CPU or Heap size limits' ); + } + @isTest private static void dmlInsert_whenTheUserCannotCreateRecords_willThrowAnException() // NOPMD: Test method name format { @@ -493,11 +516,38 @@ private without sharing class SecureDmlTest System.assertEquals( new Set{ 'Name' }, context.getValue(), 'dmlInsert, when the user cannot create records because of FLS and not all fields are switched off, will throw an exception with a context named fieldsInViolation set to the fields that were in violation minus those switched off' ); } -// when exception is supressed, will strip the fields -/* - System.assertEquals( null, accounts[0].Name, 'dmlInsert, when the user cannot create records because of FLS, will update the records to remove the fields in violation' ); - System.assertEquals( 1, accounts[0].NumberOfEmployees, 'dmlInsert, when the user cannot create records because of FLS, will ensure the supressed fields keep their values' ); -*/ + @isTest + private static void dmlInsert_whenTheUserCannotCreateRecordsDueToFlsAndNullHandler_willStripFields() // NOPMD: Test method name format + { + List accounts = new List + { + new Account( Name = 'Account1', NumberOfEmployees = 1 ), + new Account( Name = 'Account2', NumberOfEmployees = 2 ) + }; + + List strippedAccounts = new List + { + new Account(), + new Account() + }; + + Set accountFieldsBlockedByFls = new Set{ 'Name', 'NumberOfEmployees' }; + Map> removedFields = new Map>{ 'Account' => accountFieldsBlockedByFls }; + SecureDml.SecurityDecision stripsNumberOfEmployees = new SecureDml.SecurityDecision( removedFields, strippedAccounts ); + + Test.startTest(); + SecureDml dml = new TestableSecureDml() + .ignoreFlsFor( Account.sobjectType, Account.NumberOfEmployees ) + .setFlsViolationHandler( new SwallowOnFlsViolationHandler() ); + + ((TestableSecureDml)dml).stripInaccessibleSecurityDecision = stripsNumberOfEmployees; + + dml.dmlInsert( accounts ); + Test.stopTest(); + + System.assertEquals( null, accounts[0].Name, 'dmlInsert, when the user cannot create records because of FLS and handler does not throw exception, will update the records to remove the fields in violation' ); + System.assertEquals( 1, accounts[0].NumberOfEmployees, 'dmlInsert, when the user cannot create records because of FLS and handler does not throw exception, will ensure the supressed fields keep their values' ); + } private static List getAccountsInserted() { @@ -510,7 +560,8 @@ private without sharing class SecureDmlTest private static List recordsPublished; // version of SecureDml that allows us to override the checks on whether - // the current user can create / update / delete + // the current user can create / update / delete as well as the actual DML + // Overriding the DML allows the tests to run in any environment private inherited sharing class TestableSecureDml extends SecureDml { public Boolean canCreate = true; @@ -576,24 +627,17 @@ private without sharing class SecureDmlTest } } - private inherited sharing class FAccount extends sfab_FabricatedSobject + private inherited sharing class SwallowOnFlsViolationHandler implements SecureDml.FlsViolationHandler { - public FAccount() - { - super( Account.class ); - name( 'Default Name' ); - } - - public FAccount name( String name ) - { - set( Account.Name, name ); - return this; - } + public void handleInaccessibleFields( AccessType mode, SobjectType sobjectType, Set fieldsInViolation ) {} // NOPMD: intentionally left blank + } - public FAccount numberOfEmployees( Integer numberOfEmployees ) - { - set( Account.NumberOfEmployees, numberOfEmployees ); - return this; - } + private inherited sharing class SwallowOnCudViolationHandler implements SecureDml.CudViolationHandler + { + public void handleUnableToInsertRecords( List objList ) {} // NOPMD: intentionally left blank + public void handleUnableToUpdateRecords( List objList ) {} // NOPMD: intentionally left blank + public void handleUnableToDeleteRecords( List objList ) {} // NOPMD: intentionally left blank + public void handleUnableToPublishEvents( List objList ) {} // NOPMD: intentionally left blank } + } \ No newline at end of file