From be4e4c412b9aff66dc9ce3ede5bbaceae1a39b3f Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Fri, 22 Apr 2022 16:37:57 +0100 Subject: [PATCH] Added output of better exception when a circular reference is found in the Dynamic UOW Improved string output of directed graph --- .../ortoo_DynamicSobjectUnitOfWork.cls | 11 +++- .../ortoo_DynamicSobjectUnitOfWorkTest.cls | 26 +++++++++- .../default/classes/utils/DirectedGraph.cls | 32 ++++++++++++ .../default/classes/utils/ListUtils.cls | 24 +++++++++ .../default/classes/utils/SobjectUtils.cls | 10 ++-- .../classes/utils/tests/DirectedGraphTest.cls | 43 ++++++++++++++++ .../classes/utils/tests/ListUtilsTest.cls | 38 ++++++++++++++ .../classes/utils/tests/SobjectUtilsTest.cls | 50 +++++++++++++++++++ 8 files changed, 229 insertions(+), 5 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/ortoo_DynamicSobjectUnitOfWork.cls b/framework/default/ortoo-core/default/classes/fflib-extension/ortoo_DynamicSobjectUnitOfWork.cls index eea1e50162e..9121909f283 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/ortoo_DynamicSobjectUnitOfWork.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/ortoo_DynamicSobjectUnitOfWork.cls @@ -362,7 +362,16 @@ public inherited sharing class ortoo_DynamicSobjectUnitOfWork extends ortoo_Sobj private List generateOrderOfOperations() { - List childToParentTypes = graph.generateSorted(); + List childToParentTypes; + try + { + childToParentTypes = graph.generateSorted(); + } + catch ( DirectedGraph.GraphContainsCircularReferenceException e ) + { + + throw new Exceptions.ConfigurationException( 'Cannot resolve the order of work to be done for the commit, there is a circular reference in the data\n' + graph.toString(), e ); + } List parentToChildTypes = new List(); for( Integer i = childToParentTypes.size() - 1; i >= 0; i-- ) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/tests/ortoo_DynamicSobjectUnitOfWorkTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/tests/ortoo_DynamicSobjectUnitOfWorkTest.cls index 6d598ee0baf..62cd7071c54 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/tests/ortoo_DynamicSobjectUnitOfWorkTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/tests/ortoo_DynamicSobjectUnitOfWorkTest.cls @@ -523,8 +523,32 @@ private without sharing class ortoo_DynamicSobjectUnitOfWorkTest ortoo_Asserts.assertContains( 'SObject type Contact is invalid for publishing within this unit of work', exceptionMessage, 'registerPublishAfterFailureTransaction, when called with a non event, will throw an exception' ); } + @isTest + private static void commitWork_whenGivenCircularReferences_throwsAnException() // NOPMD: Test method name format + { + Contact childContact = new Contact( LastName = 'Child' ); + Contact parentContact = new Contact( LastName = 'Parent' ); + MockDml dml = new MockDml(); - // TODO: when given a circular reference + Test.startTest(); + String exceptionMessage; + try + { + ortoo_DynamicSobjectUnitOfWork uow = new ortoo_DynamicSobjectUnitOfWork( dml ); + uow.registerNew( childContact ); + uow.registerNew( parentContact ); + uow.registerRelationship( childContact, Contact.ReportsToId, parentContact ); + uow.commitWork(); + } + catch ( Exception e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'Cannot resolve the order of work to be done for the commit, there is a circular reference in the data', exceptionMessage, 'commitWork, when given circular references, will throw an exception' ); + ortoo_Asserts.assertContains( 'Contact is a child of Contact', exceptionMessage, 'commitWork, when given circular references, will throw an exception detailing the relationships' ); + } // IDml is defined as an inner class, and so cannot be mocked using Amoss at time of writing private class MockDml implements fflib_SObjectUnitOfWork.IDml diff --git a/framework/default/ortoo-core/default/classes/utils/DirectedGraph.cls b/framework/default/ortoo-core/default/classes/utils/DirectedGraph.cls index 7fa8ace1c42..a190b1119d9 100644 --- a/framework/default/ortoo-core/default/classes/utils/DirectedGraph.cls +++ b/framework/default/ortoo-core/default/classes/utils/DirectedGraph.cls @@ -92,6 +92,38 @@ public inherited sharing class DirectedGraph return sortedObjects; } + /** + * Produces a String representation of the graph + * + * @return String The graph + */ + public override String toString() + { + if ( parentsByChildren.isEmpty() ) + { + return 'Empty Graph'; + } + + List relationships = new List(); + + for ( Object thisChild : parentsByChildren.keySet() ) + { + List parents = ListUtils.convertToListOfStrings( parentsByChildren.get( thisChild ) ); + + String description; + if ( parents.size() == 0 ) + { + description = String.valueOf( thisChild ) + ' is not a child'; + } + else + { + description = String.valueOf( thisChild ) + ' is a child of ' + String.join( parents, ', ' ); + } + relationships.add( description ); + } + return String.join( relationships, '\n' ); + } + /** * A reference to the full list of nodes registered on this graph. */ diff --git a/framework/default/ortoo-core/default/classes/utils/ListUtils.cls b/framework/default/ortoo-core/default/classes/utils/ListUtils.cls index 58b1ad9ea0f..4215a8535ac 100644 --- a/framework/default/ortoo-core/default/classes/utils/ListUtils.cls +++ b/framework/default/ortoo-core/default/classes/utils/ListUtils.cls @@ -138,6 +138,30 @@ public inherited sharing class ListUtils return returnList; } + /** + * Given a Set of Objects, will return a List of those Strings + * + * Note this will only work for sets defined as Set + * + * @param Set The set of Obejcts to convert + * @return List The converted list + */ + public static List convertToListOfStrings( Set setOfObjects ) + { + if ( setOfObjects == null ) + { + return new List(); + } + + List returnList = new List(); + for ( Object thisObject : setOfObjects ) + { + returnList.add( thisObject.toString() ); + } + + return returnList; + } + /** * Given a Set of any Strings, will return a List of those Strings * diff --git a/framework/default/ortoo-core/default/classes/utils/SobjectUtils.cls b/framework/default/ortoo-core/default/classes/utils/SobjectUtils.cls index fe532737bf6..4e18d6065d4 100644 --- a/framework/default/ortoo-core/default/classes/utils/SobjectUtils.cls +++ b/framework/default/ortoo-core/default/classes/utils/SobjectUtils.cls @@ -62,11 +62,15 @@ public inherited sharing class SobjectUtils return record.getSObjectType(); } - // TODO: document - // TODO: test + /** + * Given a list of instances of SObjects, will return their SobjectTypes + * + * @param List The SObjects for which to return the SobjectTypes + * @return Set The SobjectTypes of the SObjects + */ public static Set getSobjectTypes( List records ) { - Contract.requires( records != null, 'getSobjectType called with a null records' ); + Contract.requires( records != null, 'getSobjectTypes called with a null records' ); Set types = new Set(); for ( Sobject thisRecord : records ) diff --git a/framework/default/ortoo-core/default/classes/utils/tests/DirectedGraphTest.cls b/framework/default/ortoo-core/default/classes/utils/tests/DirectedGraphTest.cls index 74ce60b9086..597e150df6b 100644 --- a/framework/default/ortoo-core/default/classes/utils/tests/DirectedGraphTest.cls +++ b/framework/default/ortoo-core/default/classes/utils/tests/DirectedGraphTest.cls @@ -181,4 +181,47 @@ private without sharing class DirectedGraphTest ortoo_Asserts.assertContains( 'addRelationship called with a parent that has not been added as a node (UnregisteredParent)', exceptionMessage, 'addRelationship, when given a parent that has not previously been added, will throw an exception' ); } + + @isTest + private static void toString_whenCalledAgainstAnGraph_returnsADescriptionOfTheGraph() // NOPMD: Test method name format + { + DirectedGraph graph = new DirectedGraph() + .addNode( 'Grandparent' ) + .addNode( 'Parent1' ) + .addNode( 'Parent2' ) + .addNode( 'Child1' ) + .addNode( 'Child2' ) + .addNode( 'Child3' ) + .addRelationship( 'Child1', 'Parent1' ) + .addRelationship( 'Child1', 'Parent2' ) + .addRelationship( 'Child2', 'Parent1' ) + .addRelationship( 'Child3', 'Parent2' ) + .addRelationship( 'Parent1', 'Grandparent' ); + + String expected = '' + + 'Grandparent is not a child\n' + + 'Parent1 is a child of Grandparent\n' + + 'Parent2 is not a child\n' + + 'Child1 is a child of Parent1, Parent2\n' + + 'Child2 is a child of Parent1\n' + + 'Child3 is a child of Parent2'; + + Test.startTest(); + String got = graph.toString(); + Test.stopTest(); + + System.assertEquals( expected, got, 'toString, when called, will return a description of the graph' ); + } + + @isTest + private static void toString_whenCalledAgainstAnEmptyGraph_returnsEmpty() // NOPMD: Test method name format + { + DirectedGraph graph = new DirectedGraph(); + + Test.startTest(); + String got = graph.toString(); + Test.stopTest(); + + System.assertEquals( 'Empty Graph', got, 'toString, when called against an empty graph, will return empty' ); + } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/utils/tests/ListUtilsTest.cls b/framework/default/ortoo-core/default/classes/utils/tests/ListUtilsTest.cls index 28f3f423552..bf73f5038ea 100644 --- a/framework/default/ortoo-core/default/classes/utils/tests/ListUtilsTest.cls +++ b/framework/default/ortoo-core/default/classes/utils/tests/ListUtilsTest.cls @@ -498,6 +498,44 @@ private without sharing class ListUtilsTest System.assertEquals( new List(), returnedList, 'convertToListOfStrings, when given null set of strings, will return an empty list' ); } + @isTest + private static void convertToListOfStrings_setObject_whenGivenASetOfStrings_returnsAListOfString() // NOPMD: Test method name format + { + Set setOfStrings = new Set{ 'one', 'two', 'three' }; + + Test.startTest(); + List returnedList = ListUtils.convertToListOfStrings( setOfStrings ); + Test.stopTest(); + + List expectedList = new List{ 'one', 'two', 'three' }; + + System.assertEquals( expectedList, returnedList, 'convertToListOfStrings, when given a set of Object, will return a list of the strings' ); + } + + @isTest + private static void convertToListOfStrings_setObject_whenGivenAnEmptySet_returnsAnEmptySet() // NOPMD: Test method name format + { + Set emptySetOfStrings = new Set(); + + Test.startTest(); + List returnedList = ListUtils.convertToListOfStrings( emptySetOfStrings ); + Test.stopTest(); + + System.assertEquals( new List(), returnedList, 'convertToListOfStrings, when given an empty set of Object, will return an empty list' ); + } + + @isTest + private static void convertToListOfStrings_setObject_whenGivenNull_returnsAnEmptyList() // NOPMD: Test method name format + { + Set nullSetOfObjects = null; + + Test.startTest(); + List returnedList = ListUtils.convertToListOfStrings( nullSetOfObjects ); + Test.stopTest(); + + System.assertEquals( new List(), returnedList, 'convertToListOfStrings, when given null set of Object, will return an empty list' ); + } + private inherited sharing class StringableThing { String name; public StringableThing( String name ) diff --git a/framework/default/ortoo-core/default/classes/utils/tests/SobjectUtilsTest.cls b/framework/default/ortoo-core/default/classes/utils/tests/SobjectUtilsTest.cls index 6be510c3fbc..6b95d646241 100644 --- a/framework/default/ortoo-core/default/classes/utils/tests/SobjectUtilsTest.cls +++ b/framework/default/ortoo-core/default/classes/utils/tests/SobjectUtilsTest.cls @@ -82,6 +82,56 @@ private without sharing class SobjectUtilsTest ortoo_Asserts.assertContains( 'getSobjectType called with a null record', exceptionMessage, 'getSobjectType, when passed a null record, will throw an exception' ); } + @isTest + private static void getSobjectTypes_whenGivenRecords_willReturnTheirTypes() // NOPMD: Test method name format + { + Set actualTypes = SobjectUtils.getSobjectTypes( new List{ new Contact(), new Account() } ); + + System.assertEquals( 2, actualTypes.size(), 'getSobjectType, when given SObject records, will return their types' ); + System.assert( actualTypes.contains( Contact.sobjectType ), 'getSobjectType, when given SObject records, will return their types - 1' ); + System.assert( actualTypes.contains( Account.sobjectType ), 'getSobjectType, when given SObject records, will return their types - 2' ); + } + + @isTest + private static void getSobjectTypes_whenPassedANullRecord_willThrowAnException() // NOPMD: Test method name format + { + Sobject nullRecord = null; + + Test.startTest(); + String exceptionMessage; + try + { + SobjectUtils.getSobjectTypes( new List{ new Contact(), new Account(), null } ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'getSobjectType called with a null record', exceptionMessage, 'getSobjectType, when passed a null record, will throw an exception' ); + } + + @isTest + private static void getSobjectTypes_whenPassedANullList_willThrowAnException() // NOPMD: Test method name format + { + List nullList = null; + + Test.startTest(); + String exceptionMessage; + try + { + SobjectUtils.getSobjectTypes( nullList ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'getSobjectTypes called with a null records', exceptionMessage, 'getSobjectTypes, when passed a null list, will throw an exception' ); + } + @isTest private static void getSobjectName_whenGivenAnSobject_willReturnTheApiNameOfIt() // NOPMD: Test method name format {