From 6a6ba9f725d778af501071276032ad859c926a10 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Tue, 8 Mar 2022 14:44:22 +0000 Subject: [PATCH 01/28] Sketching out the idea of the sobject cache --- .../{ => caching}/CachedSoqlExecutor.cls | 144 +----------------- .../CachedSoqlExecutor.cls-meta.xml | 0 .../fflib-extension/caching/ICacheAdaptor.cls | 11 ++ .../ICacheAdaptor.cls-meta.xml} | 0 .../fflib-extension/caching/NullCache.cls | 40 +++++ .../caching/NullCache.cls-meta.xml | 5 + .../fflib-extension/caching/OrgCache.cls | 42 +++++ .../caching/OrgCache.cls-meta.xml | 5 + .../fflib-extension/caching/SessionCache.cls | 42 +++++ .../caching/SessionCache.cls-meta.xml | 5 + .../fflib-extension/caching/SobjectCache.cls | 94 ++++++++++++ .../caching/SobjectCache.cls-meta.xml | 5 + .../tests/CachedSoqlExecutorTest.cls | 0 .../tests/CachedSoqlExecutorTest.cls-meta.xml | 5 + 14 files changed, 256 insertions(+), 142 deletions(-) rename framework/default/ortoo-core/default/classes/fflib-extension/{ => caching}/CachedSoqlExecutor.cls (60%) rename framework/default/ortoo-core/default/classes/fflib-extension/{ => caching}/CachedSoqlExecutor.cls-meta.xml (100%) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls rename framework/default/ortoo-core/default/classes/fflib-extension/{tests/CachedSoqlExecutorTest.cls-meta.xml => caching/ICacheAdaptor.cls-meta.xml} (100%) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls-meta.xml create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls-meta.xml create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls-meta.xml create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml rename framework/default/ortoo-core/default/classes/fflib-extension/{ => caching}/tests/CachedSoqlExecutorTest.cls (100%) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/CachedSoqlExecutor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls similarity index 60% rename from framework/default/ortoo-core/default/classes/fflib-extension/CachedSoqlExecutor.cls rename to framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls index 64870241990..c53f3c25ae6 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/CachedSoqlExecutor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls @@ -10,8 +10,9 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t { public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built // in such a way that it's never possible to get this exception + public enum CacheScope { NONE, ORG, SESSION } - CacheWrapper cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version + ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version private final static String SOQL_PARTITION_NAME = 'soql'; private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option @@ -32,8 +33,6 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t set; } - public enum CacheScope { NONE, ORG, SESSION } - public CachedSoqlExecutor setScope( CacheScope scope ) { Contract.requires( scope != null, 'setScope called with a null scope' ); @@ -172,143 +171,4 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t { return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, SOQL_PARTITION_NAME, subkey ); } - - private interface CacheWrapper - { - Boolean isACache(); - Object get( String key ); - void put( String key, Object value, Integer lifespan ); - Set getKeys(); - Boolean contains( String key ); - void remove( String key ); - String createFullyQualifiedPartitionName( String namespace, String partitionName ); - String createFullyQualifiedKey( String namespace, String partitionName, String subKey ); - } - - private class OrgCache implements CacheWrapper - { - public Boolean isACache() - { - return true; - } - - public Object get( String key ) - { - return Cache.Org.get( key ); - } - - public void put( String key, Object value, Integer lifespan ) - { - Cache.Org.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable results - } - - public Set getKeys() - { - return Cache.Org.getKeys(); - } - - public Boolean contains( String key ) - { - return Cache.Org.contains( key ); - } - - public void remove( String key ) - { - Cache.Org.remove( key ); - } - - public String createFullyQualifiedPartitionName( String namespace, String partitionName ) - { - return Cache.OrgPartition.createFullyQualifiedPartition( namespace, partitionName ); - } - - public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) - { - return Cache.OrgPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, SOQL_PARTITION_NAME, subkey ); - } - } - - private class SessionCache implements CacheWrapper - { - public Boolean isACache() - { - return true; - } - - public Object get( String key ) - { - return Cache.Session.get( key ); - } - - public void put( String key, Object value, Integer lifespan ) - { - Cache.Session.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable results - } - - public Set getKeys() - { - return Cache.Session.getKeys(); - } - - public Boolean contains( String key ) - { - return Cache.Session.contains( key ); - } - - public void remove( String key ) - { - Cache.Session.remove( key ); - } - - public String createFullyQualifiedPartitionName( String namespace, String partitionName ) - { - return Cache.SessionPartition.createFullyQualifiedPartition( namespace, partitionName ); - } - - public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) - { - return Cache.SessionPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, SOQL_PARTITION_NAME, subkey ); - } - } - - private class NullCache implements CacheWrapper - { - public Boolean isACache() - { - return false; - } - - public Object get( String key ) - { - return null; - } - - public void put( String key, Object value, Integer lifespan ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache - { - } - - public Set getKeys() - { - return new Set(); - } - - public Boolean contains( String key ) - { - return false; - } - - public void remove( String key ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache - { - } - - public String createFullyQualifiedPartitionName( String namespace, String partitionName ) - { - return namespace + '.' + partitionName; - } - - public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) - { - return namespace + '.' + partitionName + '.' + subkey; - } - } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/CachedSoqlExecutor.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls-meta.xml similarity index 100% rename from framework/default/ortoo-core/default/classes/fflib-extension/CachedSoqlExecutor.cls-meta.xml rename to framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls new file mode 100644 index 00000000000..a1d3bc557bf --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls @@ -0,0 +1,11 @@ +public interface ICacheAdaptor +{ + Boolean isACache(); + Object get( String key ); + void put( String key, Object value, Integer lifespan ); + Set getKeys(); + Boolean contains( String key ); + void remove( String key ); + String createFullyQualifiedPartitionName( String namespace, String partitionName ); + String createFullyQualifiedKey( String namespace, String partitionName, String subKey ); +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/tests/CachedSoqlExecutorTest.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls-meta.xml similarity index 100% rename from framework/default/ortoo-core/default/classes/fflib-extension/tests/CachedSoqlExecutorTest.cls-meta.xml rename to framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls new file mode 100644 index 00000000000..7c844dc2ee0 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls @@ -0,0 +1,40 @@ +public class NullCache implements ICacheAdaptor +{ + public Boolean isACache() + { + return false; + } + + public Object get( String key ) + { + return null; + } + + public void put( String key, Object value, Integer lifespan ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache + { + } + + public Set getKeys() + { + return new Set(); + } + + public Boolean contains( String key ) + { + return false; + } + + public void remove( String key ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache + { + } + + public String createFullyQualifiedPartitionName( String namespace, String partitionName ) + { + return namespace + '.' + partitionName; + } + + public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) + { + return namespace + '.' + partitionName + '.' + subkey; + } +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls new file mode 100644 index 00000000000..2a4716803f2 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -0,0 +1,42 @@ +public class OrgCache implements ICacheAdaptor +{ + public Boolean isACache() + { + return true; + } + + public Object get( String key ) + { + return Cache.Org.get( key ); + } + + public void put( String key, Object value, Integer lifespan ) + { + Cache.Org.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace + } + + public Set getKeys() + { + return Cache.Org.getKeys(); + } + + public Boolean contains( String key ) + { + return Cache.Org.contains( key ); + } + + public void remove( String key ) + { + Cache.Org.remove( key ); + } + + public String createFullyQualifiedPartitionName( String namespace, String partitionName ) + { + return Cache.OrgPartition.createFullyQualifiedPartition( namespace, partitionName ); + } + + public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) + { + return Cache.OrgPartition.createFullyQualifiedKey( namespace, partitionName, subkey ); + } +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls new file mode 100644 index 00000000000..94a1dedc2d7 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls @@ -0,0 +1,42 @@ +public class SessionCache implements ICacheAdaptor +{ + public Boolean isACache() + { + return true; + } + + public Object get( String key ) + { + return Cache.Session.get( key ); + } + + public void put( String key, Object value, Integer lifespan ) + { + Cache.Session.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace + } + + public Set getKeys() + { + return Cache.Session.getKeys(); + } + + public Boolean contains( String key ) + { + return Cache.Session.contains( key ); + } + + public void remove( String key ) + { + Cache.Session.remove( key ); + } + + public String createFullyQualifiedPartitionName( String namespace, String partitionName ) + { + return Cache.SessionPartition.createFullyQualifiedPartition( namespace, partitionName ); + } + + public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) + { + return Cache.SessionPartition.createFullyQualifiedKey( namespace, partitionName, subkey ); + } +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls new file mode 100644 index 00000000000..12160c9d986 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls @@ -0,0 +1,94 @@ +public with sharing class SobjectCache +{ + // TODO: try writing something that gets some things from the cache, then deals with the difference. + // TODO: maybe we need a return object that contains a list of the cache misses. + + public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built + // in such a way that it's never possible to get this exception + public enum CacheScope { ORG, SESSION } + + ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version + + private final static String PARTITION_NAME = 'soql'; // TODO: same partition? + private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option + + @testVisible + private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessSOQLCache'; // TODO: same permission? + + private Boolean hasAccessToCache + { + get + { + if ( hasAccessToCache == null ) + { + hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_SOQL_CACHE_PERMISSION ); + } + return hasAccessToCache; + } + set; + } + + public SobjectCache setScope( CacheScope scope ) + { + Contract.requires( scope != null, 'setScope called with a null scope' ); + + switch on scope + { + when ORG + { + cacheWrapper = new OrgCache(); + } + when SESSION + { + cacheWrapper = new SessionCache(); + } + } + + return this; + } + + // returns as many of the objects from the cache as can be returned + public CacheRetrieval get( String key, Set ids ) + { + return new CacheRetrieval(); + } + + public SobjectCache put( String key, String idField, List sobjects ) + { + return this; + } + + public SobjectCache clear( String key ) + { + return this; + } + + public SobjectCache clear( String key, Set ids ) + { + return this; + } + + public class CacheRetrieval + { + public Map cacheHits { get; private set; } + public Set cacheMisses { get; private set; } + + private CacheRetrieval() + { + cacheHits = new Map(); + cacheMisses = new Set(); + } + + private CacheRetrieval addCacheMiss( Id id ) + { + cacheMisses.add( id ); + return this; + } + + private CacheRetrieval addCacheHit( Id id, Sobject value ) + { + cacheHits.put( id, value ); + return this; + } + } +} diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/tests/CachedSoqlExecutorTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls similarity index 100% rename from framework/default/ortoo-core/default/classes/fflib-extension/tests/CachedSoqlExecutorTest.cls rename to framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + From 577e8b00d61136a2f191703741501faa4a3a7417 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Tue, 8 Mar 2022 16:19:30 +0000 Subject: [PATCH 02/28] Implemented version with a cache entry per sobject --- .../fflib-extension/caching/SobjectCache.cls | 61 ++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls index 12160c9d986..5d9712b0a63 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls @@ -1,8 +1,5 @@ public with sharing class SobjectCache { - // TODO: try writing something that gets some things from the cache, then deals with the difference. - // TODO: maybe we need a return object that contains a list of the cache misses. - public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built // in such a way that it's never possible to get this exception public enum CacheScope { ORG, SESSION } @@ -47,27 +44,77 @@ public with sharing class SobjectCache return this; } - // returns as many of the objects from the cache as can be returned public CacheRetrieval get( String key, Set ids ) { - return new CacheRetrieval(); + CacheRetrieval values = new CacheRetrieval(); + for ( Id thisId : ids ) + { + SObject thisValue = (SObject)cacheWrapper.get( createFullyQualifiedKey( key, thisId ) ); + if ( thisValue != null ) + { + values.addCacheHit( thisId, thisValue ); + } + else + { + values.addCacheMiss( thisId ); + } + } + return values; + } + + public SobjectCache put( String key, List sobjects ) + { + return put( key, 'Id', sobjects ); } public SobjectCache put( String key, String idField, List sobjects ) { + for ( Sobject thisSobject : sobjects ) + { + final String thisKey = createFullyQualifiedKey( key, (Id)thisSobject.get( idField ) ); + cacheWrapper.put( thisKey, thisSobject, CACHE_LIFESPAN_SECONDS ); + } return this; } - public SobjectCache clear( String key ) + public SobjectCache remove( String subkeyToRemove ) { + Set keys = cacheWrapper.getKeys(); + + for ( String thisKey : keys ) + { + if ( isAKeyFor( thisKey, subkeyToRemove ) ) + { + cacheWrapper.remove( thisKey ); + } + } return this; } - public SobjectCache clear( String key, Set ids ) + public SobjectCache remove( String key, Set ids ) { + for ( Id thisId : ids ) + { + cacheWrapper.remove( createFullyQualifiedKey( key, thisId ) ); + } return this; } + private Boolean isAKeyFor( String keyToCheck, String keyToCheckAgainst ) + { + return keyToCheck.startsWith( createKeyPrefix( keyToCheckAgainst ) ); + } + + private String createFullyQualifiedKey( String subKey, String id ) + { + return createKeyPrefix( subKey ) + id; + } + + private String createKeyPrefix( String subKey ) + { + return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, subkey ) + 'x'; + } + public class CacheRetrieval { public Map cacheHits { get; private set; } From 9b31ce9e3e5f06e381482d8e000d30768dbd3802 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Tue, 8 Mar 2022 16:33:48 +0000 Subject: [PATCH 03/28] Implemented a bulk version - everything in Maps... --- .../caching/BulkSobjectCache.cls | 148 ++++++++++++++++++ .../caching/BulkSobjectCache.cls-meta.xml | 5 + 2 files changed, 153 insertions(+) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls new file mode 100644 index 00000000000..06ad8949b2e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls @@ -0,0 +1,148 @@ +public with sharing class BulkSobjectCache +{ + public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built + // in such a way that it's never possible to get this exception + public enum CacheScope { ORG, SESSION } + + ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version + + private final static String PARTITION_NAME = 'soql'; // TODO: same partition? + private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option + + @testVisible + private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessSOQLCache'; // TODO: same permission? + + private Boolean hasAccessToCache + { + get + { + if ( hasAccessToCache == null ) + { + hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_SOQL_CACHE_PERMISSION ); + } + return hasAccessToCache; + } + set; + } + + public BulkSobjectCache setScope( CacheScope scope ) + { + Contract.requires( scope != null, 'setScope called with a null scope' ); + + switch on scope + { + when ORG + { + cacheWrapper = new OrgCache(); + } + when SESSION + { + cacheWrapper = new SessionCache(); + } + } + + return this; + } + + public CacheRetrieval get( String key, Set ids ) + { + CacheRetrieval values = new CacheRetrieval(); + + Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); + + if ( cachedObjects == null ) + { + return values.setCacheMisses( ids ); + } + + for ( Id thisId : ids ) + { + SObject thisValue = cachedObjects.get( thisId ); + if ( thisValue != null ) + { + values.addCacheHit( thisId, thisValue ); + } + else + { + values.addCacheMiss( thisId ); + } + } + return values; + } + + public BulkSobjectCache put( String key, List sobjects ) + { + return put( key, 'Id', sobjects ); + } + + public BulkSobjectCache put( String key, String idField, List sobjects ) + { + String fullyQualifiedKey = createFullyQualifiedKey( key ); + Map cachedObjects = (Map)cacheWrapper.get( fullyQualifiedKey ); + + if ( cachedObjects == null ) + { + cachedObjects = new Map(); + } + + for ( Sobject thisSobject : sobjects ) + { + cachedObjects.put( (Id)thisSobject.get( idField ), thisSobject ); + } + cacheWrapper.put( fullyQualifiedKey, cachedObjects, CACHE_LIFESPAN_SECONDS ); + + return this; + } + + public BulkSobjectCache remove( String key ) + { + cacheWrapper.remove( createFullyQualifiedKey( key ) ); + return this; + } + + public BulkSobjectCache remove( String key, Set ids ) + { + Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); + for ( Id thisId : ids ) + { + cachedObjects.remove( thisId ); + } + cacheWrapper.put( createFullyQualifiedKey( key ), cachedObjects, CACHE_LIFESPAN_SECONDS ); + return this; + } + + private String createFullyQualifiedKey( String subKey ) + { + return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, subkey ); + } + + public class CacheRetrieval + { + public Map cacheHits { get; private set; } + public Set cacheMisses { get; private set; } + + private CacheRetrieval() + { + cacheHits = new Map(); + cacheMisses = new Set(); + } + + private CacheRetrieval setCacheMisses( Set ids ) + { + cacheMisses = ids; + return this; + } + + private CacheRetrieval addCacheMiss( Id id ) + { + cacheMisses.add( id ); + return this; + } + + private CacheRetrieval addCacheHit( Id id, Sobject value ) + { + cacheHits.put( id, value ); + return this; + } + } +} diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + From 9e347aac34f6bf91d8dd0fedd23c9822c7c61341 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Wed, 9 Mar 2022 12:53:35 +0000 Subject: [PATCH 04/28] Made the SObjectCache more generic. Added docs --- .../caching/BulkObjectCache.cls | 352 ++++++++++++++++++ ...-meta.xml => BulkObjectCache.cls-meta.xml} | 0 .../caching/BulkSobjectCache.cls | 148 -------- .../caching/CachedSoqlExecutor.cls | 6 + 4 files changed, 358 insertions(+), 148 deletions(-) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls rename framework/default/ortoo-core/default/classes/fflib-extension/caching/{BulkSobjectCache.cls-meta.xml => BulkObjectCache.cls-meta.xml} (100%) delete mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls new file mode 100644 index 00000000000..1bd7e17aa93 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls @@ -0,0 +1,352 @@ +/** + * Provides the ability to cache the lists of objects using either the Session or Org Platform Cache + * + * Lists of objects, with a mechanism for identifying individual ones can be cached in one call. + * + * Its primary use case is the caching of SObjects, and the codebase contains shortcuts for handling SObjects in particular. + * + * Note: The lifespan of a given object is reset whenever any object in that object's list is written. + * That is, either the whole of the list is aged out, or none of it is. + */ +public with sharing class BulkObjectCache +{ + // TODO: do we need to protect against 100kb limit + // TODO: could implement a longest time since accessed, first out + // TODO: could implement a random age out + // TODO: could just ignore and invalidate the cache - check with the rest of the seniors + // TODO: Single cache + // TODO: Docs on keys + + public class InvalidIdentifierException extends Exceptions.DeveloperException {} + public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built + // in such a way that it's never possible to get this exception + public enum CacheScope { ORG, SESSION } + + /** + * The data object that is returned whenever a call to retrieve objects from the cache is made. + * + * Represents: + * * The objects that could be retieved + * * The Identifiers that resulted in cache misses + */ + public class CacheRetrieval + { + /** + * The found records, indexed by their Ids + */ + public Map cacheHits { get; private set; } + + /** + * The Ids of the records that could not be found in the cache + */ + public Set cacheMisses { get; private set; } + + private CacheRetrieval() + { + cacheHits = new Map(); + cacheMisses = new Set(); + } + + private CacheRetrieval setCacheMisses( Set ids ) + { + cacheMisses = ids; + return this; + } + + private CacheRetrieval addCacheMiss( String id ) + { + cacheMisses.add( id ); + return this; + } + + private CacheRetrieval addCacheHit( String id, Object value ) + { + cacheHits.put( id, value ); + return this; + } + } + + /** + * Interface that allows for any Object to be put into the cache. + * Defines the mechanism for getting the cache Id for the given object. + * + * E.g. an internal version is implemented: SobjectIdGetter. + * That implementation calls 'get' against the given Sobject for a configured field (which defaults to Id). + */ + public interface IdGetter + { + String getIdFor( Object objectToGetIdFor ); + } + + ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version + + private final static String PARTITION_NAME = 'soql'; // TODO: same partition? set partition? + private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option + + @testVisible + private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessSOQLCache'; // TODO: same permission? + + private Boolean hasAccessToCache + { + get + { + if ( hasAccessToCache == null ) + { + hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_SOQL_CACHE_PERMISSION ); + } + return hasAccessToCache; + } + set; + } + + /** + * Set the scope of the this cache instance. + * + * @param CacheScope The scope of this cache instance. + * @return CachedSoqlExecutor Itself, allowing for a fluent interface + */ + public BulkObjectCache setScope( CacheScope scope ) + { + Contract.requires( scope != null, 'setScope called with a null scope' ); + + switch on scope + { + when ORG + { + cacheWrapper = new OrgCache(); + } + when SESSION + { + cacheWrapper = new SessionCache(); + } + } + + return this; + } + + /** + * Get the objects with the provided String Ids from the given object list cache key + * + * @param String The base key to get the objects from + * @param Set The Ids of the objects to retrieve + * @return CacheRetrieval A representation of the objects retrieved from the cache along with a list of the cache misses + */ + public CacheRetrieval get( String key, Set ids ) + { + Contract.requires( key != null, 'get called with a null key' ); + Contract.requires( ids != null, 'get called with a null ids' ); + + CacheRetrieval values = new CacheRetrieval(); + + Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); + + if ( cachedObjects == null ) + { + return values.setCacheMisses( ids ); + } + + for ( String thisId : ids ) + { + Contract.assert( thisId != null, 'get called with a null entry in ids' ); + + Object thisValue = cachedObjects.get( thisId ); + if ( thisValue != null ) + { + values.addCacheHit( thisId, thisValue ); + } + else + { + values.addCacheMiss( thisId ); + } + } + return values; + } + + /** + * Get the objects with the provided SObject Ids from the given object list cache key + * + * @param String The base key to get the objects from + * @param Set The Ids of the SObjects to retrieve + * @return CacheRetrieval A representation of the objects retrieved from the cache along with a list of the cache misses + */ + public CacheRetrieval get( String key, Set ids ) + { + Contract.requires( ids != null, 'get called with a null ids' ); + + return get( key, SetUtils.convertToSetOfStrings( ids ) ); + } + + /** + * Get the single object with the provided String Id from the given object list cache key + * + * @param String The base key to get the objects from + * @param String The Id of the object to retrieve + * @return CacheRetrieval A representation of the object retrieved from the cache including if it was a cache miss + */ + public CacheRetrieval get( String key, String id ) + { + Contract.requires( id != null, 'get called with a null id' ); + + return get( key, new Set{ id } ); + } + + /** + * Get the single object with the provided String Id from the given object list cache key + * + * @param String The base key to get the objects from + * @param String The Id of the SObject to retrieve + * @return CacheRetrieval A representation of the object retrieved from the cache including if it was a cache miss + */ + public CacheRetrieval get( String key, Id id ) + { + Contract.requires( key != null, 'get called with a null key' ); + Contract.requires( id != null, 'get called with a null id' ); + + return get( key, new Set{ id } ); + } + + /** + * Put the given SObjects into the cache, using the given base key combined with the SObject's Id + * + * @param String The base key to put the objects into + * @param List The SObjects to store + * @return BulkObjectCache Itself, allowing for a fluent interface + */ + public BulkObjectCache put( String key, List sobjects ) + { + return put( key, 'Id', sobjects ); + } + + /** + * Put the given SObjects into the cache, using the given base key combined with the value stored + * in the SObject field as defined by idField + * + * @param String The base key to put the objects into + * @param String The field to get the individual SObject identifiers from (field value must be a non-null String) + * @param List The SObjects to store + * @return BulkObjectCache Itself, allowing for a fluent interface + */ + public BulkObjectCache put( String key, String idField, List sobjects ) + { + Contract.requires( idField != null, 'put called with a null idField' ); + return put( key, new SobjectIdGetter( idField ), sobjects ); + } + + /** + * Put the given Objects into the cache, using the given base key combined with the value that is returned from + * each object when the IdGetter is called against it + * + * @param String The base key to put the objects into + * @param IdGetter The mechanism for getting the Id from each of the given objects + * @param List The Objects to store + * @return BulkObjectCache Itself, allowing for a fluent interface + */ + public BulkObjectCache put( String key, IdGetter idGetter, List objects ) + { + Contract.requires( key != null, 'put called with a null key' ); + Contract.requires( idGetter != null, 'put called with a null idGetter' ); + Contract.requires( objects != null, 'put called with a null objects' ); + + String fullyQualifiedKey = createFullyQualifiedKey( key ); + Map cachedObjects = (Map)cacheWrapper.get( fullyQualifiedKey ); + + if ( cachedObjects == null ) + { + cachedObjects = new Map(); + } + + for ( Object thisObject : objects ) + { + Contract.assert( thisObject != null, 'put called with a null entry in objects' ); + String thisId = idGetter.getIdFor( thisObject ); + + Contract.assert( thisId != null, 'put called with an object that has a null Id: ' + thisObject ); + cachedObjects.put( thisId, thisObject ); + } + cacheWrapper.put( fullyQualifiedKey, cachedObjects, CACHE_LIFESPAN_SECONDS ); + + return this; + } + + /** + * Remove all the cached objects from the given base key. + * + * @param String The base key to remove the objects from. + * @return BulkObjectCache Itself, allowing for a fluent interface + */ + public BulkObjectCache remove( String key ) + { + Contract.requires( key != null, 'remove called with a null key' ); + cacheWrapper.remove( createFullyQualifiedKey( key ) ); + return this; + } + + /** + * Remove the cached SObjects from the given base key that been stored against the given Ids + * + * @param String The base key to remove the SObjects from. + * @param Set The cache identifiers of the SObjects to remove. + * @return BulkObjectCache Itself, allowing for a fluent interface + */ + public BulkObjectCache remove( String key, Set ids ) + { + Contract.requires( ids != null, 'remove called with a null ids' ); + return remove( key, SetUtils.convertToSetOfStrings( ids ) ); + } + + /** + * Remove the cached objects from the given base key that been stored against the given String Ids + * + * @param String The base key to remove the objects from. + * @param Set The cache identifiers of the objects to remove. + * @return BulkObjectCache Itself, allowing for a fluent interface + */ + public BulkObjectCache remove( String key, Set ids ) + { + Contract.requires( key != null, 'remove called with a null key' ); + Contract.requires( ids != null, 'remove called with a null ids' ); + + Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); + for ( String thisId : ids ) + { + Contract.assert( thisId != null, 'remove called with a null entry in ids' ); + + cachedObjects.remove( thisId ); + } + cacheWrapper.put( createFullyQualifiedKey( key ), cachedObjects, CACHE_LIFESPAN_SECONDS ); + return this; + } + + private String createFullyQualifiedKey( String subKey ) + { + return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, subkey ); + } + + private class SobjectIdGetter implements IdGetter + { + String idField; + + private SobjectIdGetter( String idField ) + { + this.idField = idField; + } + + private SobjectIdGetter() + { + this( 'Id' ); + } + + public String getIdFor( Object objectToGetIdFor ) + { + try + { + return (String)((Sobject)objectToGetIdFor).get( idField ); + } + catch ( Exception e ) + { + // TODO: set context and error code + throw new InvalidIdentifierException( 'Unable to retrieve the String Identifier from the field ' + idField + ' from the SObject: ' + objectToGetIdFor ); + } + } + } +} diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls-meta.xml similarity index 100% rename from framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls-meta.xml rename to framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls deleted file mode 100644 index 06ad8949b2e..00000000000 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkSobjectCache.cls +++ /dev/null @@ -1,148 +0,0 @@ -public with sharing class BulkSobjectCache -{ - public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built - // in such a way that it's never possible to get this exception - public enum CacheScope { ORG, SESSION } - - ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version - - private final static String PARTITION_NAME = 'soql'; // TODO: same partition? - private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option - - @testVisible - private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessSOQLCache'; // TODO: same permission? - - private Boolean hasAccessToCache - { - get - { - if ( hasAccessToCache == null ) - { - hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_SOQL_CACHE_PERMISSION ); - } - return hasAccessToCache; - } - set; - } - - public BulkSobjectCache setScope( CacheScope scope ) - { - Contract.requires( scope != null, 'setScope called with a null scope' ); - - switch on scope - { - when ORG - { - cacheWrapper = new OrgCache(); - } - when SESSION - { - cacheWrapper = new SessionCache(); - } - } - - return this; - } - - public CacheRetrieval get( String key, Set ids ) - { - CacheRetrieval values = new CacheRetrieval(); - - Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); - - if ( cachedObjects == null ) - { - return values.setCacheMisses( ids ); - } - - for ( Id thisId : ids ) - { - SObject thisValue = cachedObjects.get( thisId ); - if ( thisValue != null ) - { - values.addCacheHit( thisId, thisValue ); - } - else - { - values.addCacheMiss( thisId ); - } - } - return values; - } - - public BulkSobjectCache put( String key, List sobjects ) - { - return put( key, 'Id', sobjects ); - } - - public BulkSobjectCache put( String key, String idField, List sobjects ) - { - String fullyQualifiedKey = createFullyQualifiedKey( key ); - Map cachedObjects = (Map)cacheWrapper.get( fullyQualifiedKey ); - - if ( cachedObjects == null ) - { - cachedObjects = new Map(); - } - - for ( Sobject thisSobject : sobjects ) - { - cachedObjects.put( (Id)thisSobject.get( idField ), thisSobject ); - } - cacheWrapper.put( fullyQualifiedKey, cachedObjects, CACHE_LIFESPAN_SECONDS ); - - return this; - } - - public BulkSobjectCache remove( String key ) - { - cacheWrapper.remove( createFullyQualifiedKey( key ) ); - return this; - } - - public BulkSobjectCache remove( String key, Set ids ) - { - Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); - for ( Id thisId : ids ) - { - cachedObjects.remove( thisId ); - } - cacheWrapper.put( createFullyQualifiedKey( key ), cachedObjects, CACHE_LIFESPAN_SECONDS ); - return this; - } - - private String createFullyQualifiedKey( String subKey ) - { - return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, subkey ); - } - - public class CacheRetrieval - { - public Map cacheHits { get; private set; } - public Set cacheMisses { get; private set; } - - private CacheRetrieval() - { - cacheHits = new Map(); - cacheMisses = new Set(); - } - - private CacheRetrieval setCacheMisses( Set ids ) - { - cacheMisses = ids; - return this; - } - - private CacheRetrieval addCacheMiss( Id id ) - { - cacheMisses.add( id ); - return this; - } - - private CacheRetrieval addCacheHit( Id id, Sobject value ) - { - cacheHits.put( id, value ); - return this; - } - } -} diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls index c53f3c25ae6..0c43501af09 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls @@ -33,6 +33,12 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t set; } + /** + * Set the scope of the this cache instance. + * + * @param CacheScope The scope of this cache instance. + * @return CachedSoqlExecutor Itself, allowing for a fluent interface + */ public CachedSoqlExecutor setScope( CacheScope scope ) { Contract.requires( scope != null, 'setScope called with a null scope' ); From 72a988f4e4375f57116438ddfe759e42c7273514 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Wed, 9 Mar 2022 16:06:59 +0000 Subject: [PATCH 05/28] Moved both types of cache into the same partition (core) Only restrict access to the Org Level cache (no reason to stop session cache under any circumstances) Renamed custom permission and updated error based on that assertion Added details to invalid identifier exceptions in the object cache --- ...-meta.xml => core.cachePartition-meta.xml} | 4 +- .../default/classes/FrameworkErrorCodes.cls | 4 +- .../caching/BulkObjectCache.cls | 14 ++-- .../caching/CachedSoqlExecutor.cls | 43 ++--------- .../fflib-extension/caching/ICacheAdaptor.cls | 1 + .../fflib-extension/caching/NullCache.cls | 5 ++ .../fflib-extension/caching/OrgCache.cls | 62 ++++++++++++++++ .../fflib-extension/caching/SessionCache.cls | 5 ++ .../caching/tests/CachedSoqlExecutorTest.cls | 74 +++++-------------- ...esCanAccessCache.customPermission-meta.xml | 5 ++ ...nAccessSOQLCache.customPermission-meta.xml | 5 -- .../ortoo-core-CustomLabels.labels-meta.xml | 6 +- 12 files changed, 118 insertions(+), 110 deletions(-) rename framework/default/ortoo-core/default/cachePartitions/{soql.cachePartition-meta.xml => core.cachePartition-meta.xml} (87%) create mode 100644 framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessCache.customPermission-meta.xml delete mode 100644 framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessSOQLCache.customPermission-meta.xml diff --git a/framework/default/ortoo-core/default/cachePartitions/soql.cachePartition-meta.xml b/framework/default/ortoo-core/default/cachePartitions/core.cachePartition-meta.xml similarity index 87% rename from framework/default/ortoo-core/default/cachePartitions/soql.cachePartition-meta.xml rename to framework/default/ortoo-core/default/cachePartitions/core.cachePartition-meta.xml index c3ecab1be10..4ffcc089b1a 100644 --- a/framework/default/ortoo-core/default/cachePartitions/soql.cachePartition-meta.xml +++ b/framework/default/ortoo-core/default/cachePartitions/core.cachePartition-meta.xml @@ -1,8 +1,8 @@ - The core platform cache partition for caching soql results + The core platform cache partition true - soql + core 1 1 diff --git a/framework/default/ortoo-core/default/classes/FrameworkErrorCodes.cls b/framework/default/ortoo-core/default/classes/FrameworkErrorCodes.cls index b5fe4ee8839..691bdfbae12 100644 --- a/framework/default/ortoo-core/default/classes/FrameworkErrorCodes.cls +++ b/framework/default/ortoo-core/default/classes/FrameworkErrorCodes.cls @@ -24,8 +24,8 @@ public inherited sharing class FrameworkErrorCodes { public final static String NON_EVALUATABLE_CRITERIA = 'CRI-00000'; - public final static String CACHE_ACCESS_VIOLATION = 'CACHE-00000'; - + public final static String CACHE_ACCESS_VIOLATION = 'CACHE-00000'; + public final static String UNABLE_TO_RETRIEVE_IDENTIFIER = 'CACHE-00001'; public final static String DML_ON_INACCESSIBLE_FIELDS = '0000000'; public final static String DML_INSERT_NOT_ALLOWED = '0000001'; diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls index 1bd7e17aa93..df026937109 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls @@ -16,10 +16,10 @@ public with sharing class BulkObjectCache // TODO: could just ignore and invalidate the cache - check with the rest of the seniors // TODO: Single cache // TODO: Docs on keys + // TODO: do we need to protect against non-numeric keys? Or let the platform cache deal withit? public class InvalidIdentifierException extends Exceptions.DeveloperException {} - public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built - // in such a way that it's never possible to get this exception + public enum CacheScope { ORG, SESSION } /** @@ -80,11 +80,11 @@ public with sharing class BulkObjectCache ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version - private final static String PARTITION_NAME = 'soql'; // TODO: same partition? set partition? + private final static String PARTITION_NAME = 'core'; private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option @testVisible - private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessSOQLCache'; // TODO: same permission? + private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessCache'; // TODO: same permission? private Boolean hasAccessToCache { @@ -344,8 +344,10 @@ public with sharing class BulkObjectCache } catch ( Exception e ) { - // TODO: set context and error code - throw new InvalidIdentifierException( 'Unable to retrieve the String Identifier from the field ' + idField + ' from the SObject: ' + objectToGetIdFor ); + throw new InvalidIdentifierException( 'Unable to retrieve the String Identifier from the field ' + idField + ' from the SObject: ' + objectToGetIdFor, e ) + .setErrorCode( FrameworkErrorCodes.UNABLE_TO_RETRIEVE_IDENTIFIER ) + .addContext( 'object', objectToGetIdFor ) + .addContext( 'idField', idField ); } } } diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls index 0c43501af09..664f7393112 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls @@ -8,31 +8,13 @@ */ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of too many public methods caused by inner classes { - public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built - // in such a way that it's never possible to get this exception public enum CacheScope { NONE, ORG, SESSION } ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version - private final static String SOQL_PARTITION_NAME = 'soql'; + private final static String SOQL_PARTITION_NAME = 'core'; private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option - @testVisible - private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessSOQLCache'; - - private Boolean hasAccessToCache - { - get - { - if ( hasAccessToCache == null ) - { - hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_SOQL_CACHE_PERMISSION ); - } - return hasAccessToCache; - } - set; - } - /** * Set the scope of the this cache instance. * @@ -81,7 +63,7 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t try { - if ( hasAccessToCache ) + if ( cacheWrapper.hasAccessToCache() ) { returnValues = (List)cacheWrapper.get( key ); } @@ -89,7 +71,7 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t { if ( cacheWrapper.isACache() ) { - System.debug( LoggingLevel.INFO, 'Opportunity to use Platform Cache skipped since user does not have required permission (custom permission: ' + CAN_ACCESS_SOQL_CACHE_PERMISSION + ')' ); + System.debug( LoggingLevel.INFO, 'Opportunity to use Platform Cache skipped since user does not have required permission' ); // TODO: can we get the permission name into this? } } } @@ -101,7 +83,7 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t if ( returnValues == null ) { - if ( hasAccessToCache && cacheWrapper.isACache() ) + if ( cacheWrapper.hasAccessToCache() && cacheWrapper.isACache() ) { System.debug( LoggingLevel.INFO, 'Platform Cache miss when running the SOQL: ' + soql ); } @@ -110,7 +92,7 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t try { - if ( hasAccessToCache ) + if ( cacheWrapper.hasAccessToCache() ) { cacheWrapper.put( key, returnValues, CACHE_LIFESPAN_SECONDS ); } @@ -133,14 +115,6 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t public void clearCacheFor( String soql ) { Contract.requires( soql != null, 'clearCacheFor called with a null soql' ); - - if ( ! hasAccessToCache ) - { - throw new CacheAccessViolationException( Label.ortoo_core_soql_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'clearCacheFor' ) - .addContext( 'soql', soql ); - } cacheWrapper.remove( generateKey( soql ) ); } @@ -149,13 +123,6 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t */ public void clearAllCache() { - if ( ! hasAccessToCache ) - { - throw new CacheAccessViolationException( Label.ortoo_core_soql_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'clearAllCache' ); - } - String fullSoqlPartitionName = cacheWrapper.createFullyQualifiedPartitionName( PackageUtils.NAMESPACE_PREFIX, SOQL_PARTITION_NAME ); for ( String thisKey : cacheWrapper.getKeys() ) { diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls index a1d3bc557bf..891b66c6fe3 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls @@ -1,5 +1,6 @@ public interface ICacheAdaptor { + Boolean hasAccessToCache(); Boolean isACache(); Object get( String key ); void put( String key, Object value, Integer lifespan ); diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls index 7c844dc2ee0..06c743cee45 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls @@ -1,5 +1,10 @@ public class NullCache implements ICacheAdaptor { + public Boolean hasAccessToCache() + { + return false; + } + public Boolean isACache() { return false; diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index 2a4716803f2..67f5436cc41 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -1,5 +1,29 @@ public class OrgCache implements ICacheAdaptor { + public class AccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built + // in such a way that it's never possible to get this exception + + @testVisible + private final static String CAN_ACCESS_CACHE_PERMISSION = 'ProcessesCanAccessCache'; + + public Boolean hasAccessToCache + { + get + { + if ( hasAccessToCache == null ) + { + hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_CACHE_PERMISSION ); + } + return hasAccessToCache; + } + private set; + } + + public Boolean hasAccessToCache() + { + return this.hasAccessToCache; + } + public Boolean isACache() { return true; @@ -7,26 +31,64 @@ public class OrgCache implements ICacheAdaptor public Object get( String key ) { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'get' ) + .addContext( 'key', key ); + } + return Cache.Org.get( key ); } public void put( String key, Object value, Integer lifespan ) { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'put' ) + .addContext( 'key', key ) + .addContext( 'value', value ); + } + Cache.Org.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace } public Set getKeys() { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'getKeys' ); + } + return Cache.Org.getKeys(); } public Boolean contains( String key ) { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'contains' ) + .addContext( 'key', key ); + } return Cache.Org.contains( key ); } public void remove( String key ) { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'remove' ) + .addContext( 'key', key ); + } Cache.Org.remove( key ); } diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls index 94a1dedc2d7..511496b09c5 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls @@ -1,5 +1,10 @@ public class SessionCache implements ICacheAdaptor { + public Boolean hasAccessToCache() + { + return true; + } + public Boolean isACache() { return true; diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls index 7898d997d5f..a5245ba7ac2 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls @@ -110,13 +110,13 @@ private without sharing class CachedSoqlExecutorTest { executor.clearAllCache(); } - catch ( CachedSoqlExecutor.CacheAccessViolationException e ) + catch ( OrgCache.AccessViolationException e ) { exceptionThrown = e; } Test.stopTest(); - ortoo_Asserts.assertContains( Label.ortoo_core_soql_cache_access_violation, exceptionThrown?.getMessage(), 'clearAllCache, when the user does not have access to the cache, will throw an exception' ); + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'clearAllCache, when the user does not have access to the cache, will throw an exception' ); } @isTest @@ -192,13 +192,13 @@ private without sharing class CachedSoqlExecutorTest { executor.clearCacheFor( soqlStatement ); } - catch ( CachedSoqlExecutor.CacheAccessViolationException e ) + catch ( OrgCache.AccessViolationException e ) { exceptionThrown = e; } Test.stopTest(); - ortoo_Asserts.assertContains( Label.ortoo_core_soql_cache_access_violation, exceptionThrown?.getMessage(), 'clearCacheFor, when the user does not have access to the cache, will throw an exception' ); + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'clearCacheFor, when the user does not have access to the cache, will throw an exception' ); } @isTest @@ -257,7 +257,7 @@ private without sharing class CachedSoqlExecutorTest } @isTest - private static void query_session_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesTwoSoqlStatements() // NOPMD: Test method name format + private static void query_session_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesOneSoqlStatement() // NOPMD: Test method name format { String soqlStatement = 'SELECT Id FROM Account'; @@ -271,8 +271,8 @@ private without sharing class CachedSoqlExecutorTest Integer soqlCalls = Limits.getQueries(); Test.stopTest(); - System.assertEquals( 2, soqlCalls, 'query, when called twice by a user with no access to the cache, will issue two SOQL statements' ); - System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with not access to the cache, returns the same results in both calls' ); + System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with no access to the org cache, will issue one SOQL statement' ); + System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with no access to the cache, returns the same results in both calls' ); } @isTest @@ -314,25 +314,17 @@ private without sharing class CachedSoqlExecutorTest } @isTest - private static void clearAllCache_session_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + private static void clearAllCache_session_whenTheUserDoesNotHaveAccessToTheOrgCache_willNotThrowAnException() // NOPMD: Test method name format { setupAccessToSoqlCache( false ); CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); Test.startTest(); - Exception exceptionThrown; - try - { - executor.clearAllCache(); - } - catch ( CachedSoqlExecutor.CacheAccessViolationException e ) - { - exceptionThrown = e; - } + executor.clearAllCache(); Test.stopTest(); - ortoo_Asserts.assertContains( Label.ortoo_core_soql_cache_access_violation, exceptionThrown?.getMessage(), 'clearAllCache, when the user does not have access to the cache, will throw an exception' ); + System.assert( true, 'clearAllCache, when the user does not have access to the org cache, will not throw an exception' ); } @isTest @@ -394,7 +386,7 @@ private without sharing class CachedSoqlExecutorTest } @isTest - private static void clearCacheFor_session_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + private static void clearCacheFor_session_whenTheUserDoesNotHaveAccessToTheOrgCache_willNotThrowAnException() // NOPMD: Test method name format { String soqlStatement = 'SELECT Id FROM Account'; @@ -403,19 +395,11 @@ private without sharing class CachedSoqlExecutorTest CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); Test.startTest(); - Exception exceptionThrown; - try - { - executor.clearCacheFor( soqlStatement ); - } - catch ( CachedSoqlExecutor.CacheAccessViolationException e ) - { - exceptionThrown = e; - } + executor.clearCacheFor( soqlStatement ); Test.stopTest(); - ortoo_Asserts.assertContains( Label.ortoo_core_soql_cache_access_violation, exceptionThrown?.getMessage(), 'clearCacheFor, when the user does not have access to the cache, will throw an exception' ); - } + System.assert( true, 'clearCacheFor, when the user does not have access to the org cache, will not throw an exception' ); + } @isTest private static void query_session_whenRanFor100Queries_willNotThrowAnException() @@ -515,25 +499,17 @@ private without sharing class CachedSoqlExecutorTest } @isTest - private static void clearAllCache_none_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + private static void clearAllCache_none_whenTheUserDoesNotHaveAccessToTheOrgCache_willNotThrowAnException() // NOPMD: Test method name format { setupAccessToSoqlCache( false ); CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); Test.startTest(); - Exception exceptionThrown; - try - { - executor.clearAllCache(); - } - catch ( CachedSoqlExecutor.CacheAccessViolationException e ) - { - exceptionThrown = e; - } + executor.clearAllCache(); Test.stopTest(); - ortoo_Asserts.assertContains( Label.ortoo_core_soql_cache_access_violation, exceptionThrown?.getMessage(), 'clearAllCache against a none cache, when the user does not have access to the cache, will throw an exception' ); + System.assert( true, 'clearAllCache against a none cache, when called by a user that does not have access to the org cache, will not throw an exception' ); } @isTest @@ -556,7 +532,6 @@ private without sharing class CachedSoqlExecutorTest System.assertEquals( 1, soqlCalls, 'clearCacheFor against a none cache, does not affect the number of SOQL statements issued' ); } - @isTest private static void clearCacheFor_none_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format { @@ -567,18 +542,10 @@ private without sharing class CachedSoqlExecutorTest CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); Test.startTest(); - Exception exceptionThrown; - try - { - executor.clearCacheFor( soqlStatement ); - } - catch ( CachedSoqlExecutor.CacheAccessViolationException e ) - { - exceptionThrown = e; - } + executor.clearCacheFor( soqlStatement ); Test.stopTest(); - ortoo_Asserts.assertContains( Label.ortoo_core_soql_cache_access_violation, exceptionThrown?.getMessage(), 'clearCacheFor against a none cache, when the user does not have access to the cache, will throw an exception' ); + System.assert( true, 'clearCacheFor against a none cache, when called by a user that does not have access to the org cache, will not throw an exception' ); } @isTest @@ -595,7 +562,6 @@ private without sharing class CachedSoqlExecutorTest CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); Test.startTest(); - // Run each statement once - this is the maximum we can do for ( String thisSoqlStatement : soqlStatements ) { @@ -610,7 +576,7 @@ private without sharing class CachedSoqlExecutorTest { ApplicationMockRegistrar.registerMockService( IPermissionsService.class ) .when( 'hasPermission' ) - .withParameter( CachedSoqlExecutor.CAN_ACCESS_SOQL_CACHE_PERMISSION ) + .withParameter( OrgCache.CAN_ACCESS_CACHE_PERMISSION ) .returns( accessToCache ); } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessCache.customPermission-meta.xml b/framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessCache.customPermission-meta.xml new file mode 100644 index 00000000000..26f1fda7333 --- /dev/null +++ b/framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessCache.customPermission-meta.xml @@ -0,0 +1,5 @@ + + + Allows the user's processes to access the Org Level Platform cache. Is limited since this may allow the user's process to access data that their permissions would not otherwise allow. + + \ No newline at end of file diff --git a/framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessSOQLCache.customPermission-meta.xml b/framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessSOQLCache.customPermission-meta.xml deleted file mode 100644 index 00cd33828fe..00000000000 --- a/framework/default/ortoo-core/default/customPermissions/ProcessesCanAccessSOQLCache.customPermission-meta.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - Allows the user's processes to access the SOQL cache. Is limited since this may allow the user's process to access data that their permissions would not otherwise allow. - - \ No newline at end of file diff --git a/framework/default/ortoo-core/default/labels/ortoo-core-CustomLabels.labels-meta.xml b/framework/default/ortoo-core/default/labels/ortoo-core-CustomLabels.labels-meta.xml index c2ee996a1a0..262bfe59000 100644 --- a/framework/default/ortoo-core/default/labels/ortoo-core-CustomLabels.labels-meta.xml +++ b/framework/default/ortoo-core/default/labels/ortoo-core-CustomLabels.labels-meta.xml @@ -197,10 +197,10 @@ Problem occured when configuring which fields are orderable. Table sorting is disabled. - ortoo_core_soql_cache_access_violation + ortoo_core_org_cache_access_violation en_US false - Error when unauthorised user performs an action against Org Level SOQL Cache. - Attempted to access the Org Level SOQL Cache when the user does not have permission. + Error when unauthorised user performs an action against the Org Level Cache. + Attempted to access the Org Level Platform Cache when the user does not have permission. \ No newline at end of file From 48259c60902629cedbb21ce662d6ff12ecc55087 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Wed, 9 Mar 2022 16:33:25 +0000 Subject: [PATCH 06/28] Removed redundant code --- .../caching/BulkObjectCache.cls | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls index df026937109..db35ce35f41 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls @@ -14,9 +14,9 @@ public with sharing class BulkObjectCache // TODO: could implement a longest time since accessed, first out // TODO: could implement a random age out // TODO: could just ignore and invalidate the cache - check with the rest of the seniors - // TODO: Single cache // TODO: Docs on keys - // TODO: do we need to protect against non-numeric keys? Or let the platform cache deal withit? + + // TODO: hide createFullyQualifiedPartitionName and createFullyQualifiedKey public class InvalidIdentifierException extends Exceptions.DeveloperException {} @@ -83,22 +83,6 @@ public with sharing class BulkObjectCache private final static String PARTITION_NAME = 'core'; private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option - @testVisible - private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessCache'; // TODO: same permission? - - private Boolean hasAccessToCache - { - get - { - if ( hasAccessToCache == null ) - { - hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_SOQL_CACHE_PERMISSION ); - } - return hasAccessToCache; - } - set; - } - /** * Set the scope of the this cache instance. * From 5d00e7fdec8ae07f28b633a276f572c71cf0b51f Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Wed, 9 Mar 2022 16:34:09 +0000 Subject: [PATCH 07/28] Removed need to pass the namespace in to get the qualified keys Added docs to more of the cache classes --- .../caching/BulkObjectCache.cls | 4 +- .../caching/CachedSoqlExecutor.cls | 6 +- .../fflib-extension/caching/ICacheAdaptor.cls | 13 +- .../fflib-extension/caching/NullCache.cls | 17 +- .../fflib-extension/caching/OrgCache.cls | 233 +++++++++++------- .../fflib-extension/caching/SessionCache.cls | 133 ++++++---- 6 files changed, 266 insertions(+), 140 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls index db35ce35f41..4133a2d33a9 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls @@ -301,9 +301,9 @@ public with sharing class BulkObjectCache return this; } - private String createFullyQualifiedKey( String subKey ) + private String createFullyQualifiedKey( String key ) { - return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, subkey ); + return cacheWrapper.createFullyQualifiedKey( PARTITION_NAME, key ); } private class SobjectIdGetter implements IdGetter diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls index 664f7393112..e6c64ae341a 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls @@ -12,7 +12,7 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version - private final static String SOQL_PARTITION_NAME = 'core'; + private final static String PARTITION_NAME = 'core'; private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option /** @@ -123,7 +123,7 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t */ public void clearAllCache() { - String fullSoqlPartitionName = cacheWrapper.createFullyQualifiedPartitionName( PackageUtils.NAMESPACE_PREFIX, SOQL_PARTITION_NAME ); + String fullSoqlPartitionName = cacheWrapper.createFullyQualifiedPartitionName( PARTITION_NAME ); for ( String thisKey : cacheWrapper.getKeys() ) { String qualifiedKey = qualifiedKey( thisKey ); @@ -142,6 +142,6 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t private String qualifiedKey( String subkey ) { - return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, SOQL_PARTITION_NAME, subkey ); + return cacheWrapper.createFullyQualifiedKey( PARTITION_NAME, subkey ); } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls index 891b66c6fe3..512d3f5d0bc 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls @@ -1,3 +1,12 @@ +/** + * Defines an interface that can be used to wrap / adapt a caching mechanism. + * + * Specifically designed to support the wrapping of the static Apex methods for: + * * Cache.Org / Cache.OrgPartition + * * Cache.Session / Cache.SessionPartition + * + * This allows for the types of cache to be used interchangably / and dynamically + */ public interface ICacheAdaptor { Boolean hasAccessToCache(); @@ -7,6 +16,6 @@ public interface ICacheAdaptor Set getKeys(); Boolean contains( String key ); void remove( String key ); - String createFullyQualifiedPartitionName( String namespace, String partitionName ); - String createFullyQualifiedKey( String namespace, String partitionName, String subKey ); + String createFullyQualifiedPartitionName( String partitionName ); + String createFullyQualifiedKey( String partitionName, String key ); } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls index 06c743cee45..0007e590817 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls @@ -1,8 +1,15 @@ +/** + * An implementation of the ICacheAdaptor that is benign. + * + * Allows for the use of code that will automatically interact with a cache and be able to switch that off dynamically and simply. + * + * All users are assumed to be allowed to use the cache, but it describes itself as 'not a cache' and effectively does nothing. + */ public class NullCache implements ICacheAdaptor { public Boolean hasAccessToCache() { - return false; + return true; } public Boolean isACache() @@ -33,13 +40,13 @@ public class NullCache implements ICacheAdaptor { } - public String createFullyQualifiedPartitionName( String namespace, String partitionName ) + public String createFullyQualifiedPartitionName( String partitionName ) { - return namespace + '.' + partitionName; + return partitionName; } - public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) + public String createFullyQualifiedKey( String partitionName, String subKey ) { - return namespace + '.' + partitionName + '.' + subkey; + return partitionName + '.' + subkey; } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index 67f5436cc41..f17aa3b15c4 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -1,12 +1,20 @@ +/** + * An implementation of the ICacheAdaptor that utilises the Org Level Platform Cache. + * + * Due to the shared nature of the cache, all interactions with the cache require the user to have the + * custom permission as defined in 'CAN_ACCESS_CACHE_PERMISSION'. + * + * Attempting to access the cache without this permission will result in an OrgCache.AccessViolationException + */ public class OrgCache implements ICacheAdaptor { public class AccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built // in such a way that it's never possible to get this exception - @testVisible + @testVisible private final static String CAN_ACCESS_CACHE_PERMISSION = 'ProcessesCanAccessCache'; - public Boolean hasAccessToCache + private Boolean hasAccessToCache { get { @@ -19,86 +27,143 @@ public class OrgCache implements ICacheAdaptor private set; } - public Boolean hasAccessToCache() - { - return this.hasAccessToCache; - } - - public Boolean isACache() - { - return true; - } - - public Object get( String key ) - { - if ( ! hasAccessToCache ) - { - throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'get' ) - .addContext( 'key', key ); - } - - return Cache.Org.get( key ); - } - - public void put( String key, Object value, Integer lifespan ) - { - if ( ! hasAccessToCache ) - { - throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'put' ) - .addContext( 'key', key ) - .addContext( 'value', value ); - } - - Cache.Org.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace - } - - public Set getKeys() - { - if ( ! hasAccessToCache ) - { - throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'getKeys' ); - } - - return Cache.Org.getKeys(); - } - - public Boolean contains( String key ) - { - if ( ! hasAccessToCache ) - { - throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'contains' ) - .addContext( 'key', key ); - } - return Cache.Org.contains( key ); - } - - public void remove( String key ) - { - if ( ! hasAccessToCache ) - { - throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'remove' ) - .addContext( 'key', key ); - } - Cache.Org.remove( key ); - } - - public String createFullyQualifiedPartitionName( String namespace, String partitionName ) - { - return Cache.OrgPartition.createFullyQualifiedPartition( namespace, partitionName ); - } - - public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) - { - return Cache.OrgPartition.createFullyQualifiedKey( namespace, partitionName, subkey ); - } + /** + * States if this user has access to the cache - I.E. has the custom permission defined in CAN_ACCESS_CACHE_PERMISSION + * + * @return Boolean States if the current user has access to the cache + */ + public Boolean hasAccessToCache() + { + return this.hasAccessToCache; + } + + /** + * States that this is a cache + * + * @return Boolean True, stating that this instance represents a true cache + */ + public Boolean isACache() + { + return true; + } + + /** + * Retrieve the cache entry with the given key. + * + * If the user does not have access to the cache, will throw an AccessViolationException. + * If the entry does not exist in the cache, will return null + * + * @param String The key for the object to retrieve + * @return Object The cached object, if it exists + */ + public Object get( String key ) + { + Contract.requires( key != null, 'get called with a null key' ); + + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'get' ) + .addContext( 'key', key ); + } + + return Cache.Org.get( key ); + } + + /** + * Put the stated value into the stated key for the specified duration (in seconds) + * + * If the user does not have access to the cache, will throw an AccessViolationException. + * + * @param String The key to use for the storage of the object + * @param Object The object to store + * @param Integer The lifespan of the object within the cache, in seconds + * @return Object The cached object, if it exists + */ + public void put( String key, Object value, Integer lifespan ) + { + Contract.requires( key != null, 'put called with a null key' ); + Contract.requires( lifespan != null, 'put called with a null lifespan' ); + Contract.requires( lifespan >= 0, 'put called with a negaitve lifespan' ); + + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'put' ) + .addContext( 'key', key ) + .addContext( 'value', value ); + } + + Cache.Org.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace + } + + /** + * Retrieve a set of the current keys of objects stored in this cache. + * + * If the user does not have access to the cache, will throw an AccessViolationException. + * + * @return Set The keys that currently exist in the cache + */ + public Set getKeys() + { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'getKeys' ); + } + + return Cache.Org.getKeys(); + } + + /** + * States if the cache currently contains an object in the given key. + * + * If the user does not have access to the cache, will throw an AccessViolationException. + * + * @param String The key for the object to look for + * @return Boolean Whether the key exists in the cache + */ + public Boolean contains( String key ) + { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'contains' ) + .addContext( 'key', key ); + } + return Cache.Org.contains( key ); + } + + /** + * Instucts the cache to remove the object at the given key + * + * If the user does not have access to the cache, will throw an AccessViolationException. + * + * @param String The key to remove + */ + public void remove( String key ) + { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'remove' ) + .addContext( 'key', key ); + } + Cache.Org.remove( key ); + } + + public String createFullyQualifiedPartitionName( String partitionName ) + { + return Cache.OrgPartition.createFullyQualifiedPartition( PackageUtils.NAMESPACE_PREFIX, partitionName ); + } + + public String createFullyQualifiedKey( String partitionName, String subKey ) + { + return Cache.OrgPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, partitionName, subkey ); + } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls index 511496b09c5..4bfc5f4bfec 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls @@ -1,47 +1,92 @@ +/** + * An implementation of the ICacheAdaptor that utilises the Session Level Platform Cache. + * + * All users are assumed to have access to the cache. + */ public class SessionCache implements ICacheAdaptor { - public Boolean hasAccessToCache() - { - return true; - } - - public Boolean isACache() - { - return true; - } - - public Object get( String key ) - { - return Cache.Session.get( key ); - } - - public void put( String key, Object value, Integer lifespan ) - { - Cache.Session.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace - } - - public Set getKeys() - { - return Cache.Session.getKeys(); - } - - public Boolean contains( String key ) - { - return Cache.Session.contains( key ); - } - - public void remove( String key ) - { - Cache.Session.remove( key ); - } - - public String createFullyQualifiedPartitionName( String namespace, String partitionName ) - { - return Cache.SessionPartition.createFullyQualifiedPartition( namespace, partitionName ); - } - - public String createFullyQualifiedKey( String namespace, String partitionName, String subKey ) - { - return Cache.SessionPartition.createFullyQualifiedKey( namespace, partitionName, subkey ); - } + /** + * States if this user has access to the cache - which is always true + * + * @return Boolean True - Stating that the current user has access to the cache + */ + public Boolean hasAccessToCache() + { + return true; + } + + /** + * States that this is a cache + * + * @return Boolean True, stating that this instance represents a true cache + */ + public Boolean isACache() + { + return true; + } + + /** + * Retrieve the cache entry with the given key. + * + * @param String The key for the object to retrieve + * @return Object The cached object, if it exists + */ + public Object get( String key ) + { + return Cache.Session.get( key ); + } + + /** + * Put the stated value into the stated key for the specified duration (in seconds) + * + * @param String The key to use for the storage of the object + * @param Object The object to store + * @param Integer The lifespan of the object within the cache, in seconds + * @return Object The cached object, if it exists + */ + public void put( String key, Object value, Integer lifespan ) + { + Cache.Session.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace + } + + /** + * Retrieve a set of the current keys of objects stored in this cache. + * + * @return Set The keys that currently exist in the cache + */ + public Set getKeys() + { + return Cache.Session.getKeys(); + } + + /** + * States if the cache currently contains an object in the given key. + * + * @param String The key for the object to look for + * @return Boolean Whether the key exists in the cache + */ + public Boolean contains( String key ) + { + return Cache.Session.contains( key ); + } + + /** + * Instucts the cache to remove the object at the given key + * + * @param String The key to remove + */ + public void remove( String key ) + { + Cache.Session.remove( key ); + } + + public String createFullyQualifiedPartitionName( String partitionName ) + { + return Cache.SessionPartition.createFullyQualifiedPartition( PackageUtils.NAMESPACE_PREFIX, partitionName ); + } + + public String createFullyQualifiedKey( String partitionName, String subKey ) + { + return Cache.SessionPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, partitionName, subkey ); + } } \ No newline at end of file From 3658177adc6d2329df9ffd9eab33dbfda43ea1f3 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 09:28:19 +0000 Subject: [PATCH 08/28] Abstraction of permissions check for access to the platform cache --- .../classes/fflib-extension/caching/OrgCache.cls | 5 +---- .../fflib-extension/caching/SobjectCache.cls | 8 +++----- .../caching/tests/CachedSoqlExecutorTest.cls | 3 +-- .../permissions-service/IPermissionsService.cls | 2 +- .../permissions-service/PermissionsService.cls | 7 +++---- .../permissions-service/PermissionsServiceImpl.cls | 13 ++++++++++++- .../tests/PermissionsServiceImplTest.cls | 14 +++----------- 7 files changed, 24 insertions(+), 28 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index f17aa3b15c4..a4d17b4ba06 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -11,16 +11,13 @@ public class OrgCache implements ICacheAdaptor public class AccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built // in such a way that it's never possible to get this exception - @testVisible - private final static String CAN_ACCESS_CACHE_PERMISSION = 'ProcessesCanAccessCache'; - private Boolean hasAccessToCache { get { if ( hasAccessToCache == null ) { - hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_CACHE_PERMISSION ); + hasAccessToCache = PermissionsService.hasAccessToCorePlatformCache(); } return hasAccessToCache; } diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls index 5d9712b0a63..5d2b88601e3 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls @@ -1,3 +1,4 @@ +// TODO: defunt public with sharing class SobjectCache { public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built @@ -9,16 +10,13 @@ public with sharing class SobjectCache private final static String PARTITION_NAME = 'soql'; // TODO: same partition? private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option - @testVisible - private final static String CAN_ACCESS_SOQL_CACHE_PERMISSION = 'ProcessesCanAccessSOQLCache'; // TODO: same permission? - private Boolean hasAccessToCache { get { if ( hasAccessToCache == null ) { - hasAccessToCache = PermissionsService.hasPermission( CAN_ACCESS_SOQL_CACHE_PERMISSION ); + hasAccessToCache = PermissionsService.hasAccessToCorePlatformCache(); } return hasAccessToCache; } @@ -112,7 +110,7 @@ public with sharing class SobjectCache private String createKeyPrefix( String subKey ) { - return cacheWrapper.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, subkey ) + 'x'; + return cacheWrapper.createFullyQualifiedKey( PARTITION_NAME, subkey ) + 'x'; } public class CacheRetrieval diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls index a5245ba7ac2..1ead66f5e98 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls @@ -575,8 +575,7 @@ private without sharing class CachedSoqlExecutorTest private static void setupAccessToSoqlCache( Boolean accessToCache ) { ApplicationMockRegistrar.registerMockService( IPermissionsService.class ) - .when( 'hasPermission' ) - .withParameter( OrgCache.CAN_ACCESS_CACHE_PERMISSION ) + .when( 'hasAccessToCorePlatformCache' ) .returns( accessToCache ); } } \ No newline at end of file diff --git a/framework/default/standard-services/default/classes/services/permissions-service/IPermissionsService.cls b/framework/default/standard-services/default/classes/services/permissions-service/IPermissionsService.cls index 0a15f3a4b8a..857226e21fe 100644 --- a/framework/default/standard-services/default/classes/services/permissions-service/IPermissionsService.cls +++ b/framework/default/standard-services/default/classes/services/permissions-service/IPermissionsService.cls @@ -1,4 +1,4 @@ public interface IPermissionsService { - Boolean hasPermission( String customPermissionName ); + Boolean hasAccessToCorePlatformCache(); } \ No newline at end of file diff --git a/framework/default/standard-services/default/classes/services/permissions-service/PermissionsService.cls b/framework/default/standard-services/default/classes/services/permissions-service/PermissionsService.cls index 34f7dc8ef9f..b432ec7268e 100644 --- a/framework/default/standard-services/default/classes/services/permissions-service/PermissionsService.cls +++ b/framework/default/standard-services/default/classes/services/permissions-service/PermissionsService.cls @@ -4,14 +4,13 @@ public with sharing class PermissionsService { /** - * States if the user has the custom permission with the given API name. + * States if the user has rights to access the core platform cache * - * @param String The API name of the custom permission to check the assignment of. * @return Boolean Does the current user have the stated permission. */ - public static Boolean hasPermission( String customPermissionName ) + public static Boolean hasAccessToCorePlatformCache() { - return service().hasPermission( customPermissionName ); + return service().hasAccessToCorePlatformCache(); } private static IPermissionsService service() diff --git a/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls b/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls index d7b989a4ae1..deda1113d0c 100644 --- a/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls +++ b/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls @@ -1,6 +1,17 @@ public with sharing class PermissionsServiceImpl implements IPermissionsService { - public Boolean hasPermission( String customPermissionName ) + public Boolean hasAccessToCorePlatformCache() + { + return hasCustomPermission( 'customPermissionName' ); + } + + /** + * States if the user has the custom permission with the given API name. + * + * @param String The API name of the custom permission to check the assignment of. + * @return Boolean Does the current user have the stated permission. + */ + private Boolean hasCustomPermission( String customPermissionName ) { return FeatureManagement.checkPermission( customPermissionName ); } diff --git a/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls b/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls index d809b4204ea..e84d639d11b 100644 --- a/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls +++ b/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls @@ -2,18 +2,10 @@ private with sharing class PermissionsServiceImplTest { @isTest - private static void hasPermission_givenACustomPermissionThatExists_doesNotThrowAnException() // NOPMD: test method format + private static void hasAccessToCorePlatformCache_doesNotThrowAnException() // NOPMD: test method format { - Boolean hasPermission = PermissionsService.hasPermission( 'ProcessesCanAccessSOQLCache' ); + Boolean hasPermission = PermissionsService.hasAccessToCorePlatformCache(); - System.assert( true, 'hasPermission, when given a custom permission that exists, does not throw an exception' ); - } - - @isTest - private static void hasPermission_givenACustomPermissionThatDoesNotExists_returnsFalse() // NOPMD: test method format - { - Boolean hasPermission = PermissionsService.hasPermission( 'invalid-permission-that-does-not-exist' ); - - System.assertEquals( false, hasPermission, 'hasPermission, when given a custom permission that does not exist, returns false' ); + System.assertNotEquals( null, hasPermission, 'hasAccessToCorePlatformCache, does not throw an exception, and returns a value' ); } } \ No newline at end of file From 377619d5666dc5c377a8bcf9f2c59a2dfaeec471 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 14:21:05 +0000 Subject: [PATCH 09/28] Changed permissions service test to test the actual implementation (less risky) --- .../services/permissions-service/PermissionsServiceImpl.cls | 2 +- .../permissions-service/tests/PermissionsServiceImplTest.cls | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls b/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls index deda1113d0c..e70daaa65d2 100644 --- a/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls +++ b/framework/default/standard-services/default/classes/services/permissions-service/PermissionsServiceImpl.cls @@ -2,7 +2,7 @@ public with sharing class PermissionsServiceImpl implements IPermissionsService { public Boolean hasAccessToCorePlatformCache() { - return hasCustomPermission( 'customPermissionName' ); + return hasCustomPermission( 'ProcessesCanAccessCache' ); } /** diff --git a/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls b/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls index e84d639d11b..bd7f30ef4ed 100644 --- a/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls +++ b/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls @@ -4,7 +4,7 @@ private with sharing class PermissionsServiceImplTest @isTest private static void hasAccessToCorePlatformCache_doesNotThrowAnException() // NOPMD: test method format { - Boolean hasPermission = PermissionsService.hasAccessToCorePlatformCache(); + Boolean hasPermission = new PermissionsServiceImpl().hasAccessToCorePlatformCache(); System.assertNotEquals( null, hasPermission, 'hasAccessToCorePlatformCache, does not throw an exception, and returns a value' ); } From 8a5dfdacd94cff179880547429bbd520c9e438fc Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 14:23:16 +0000 Subject: [PATCH 10/28] Renamed the BulkObjectCache to the correct ObjectCache Removed the defunct SobjectCache Hid references to the partition specification Removed 'getKeys' as this should no longer be needed Removed 'clearAll' from SOQL cache, as this in an inappropriate level (it would clear all Org level cache) Ensure that all classes have inherited sharing --- .../caching/CachedSoqlExecutor.cls | 25 +- .../fflib-extension/caching/ICacheAdaptor.cls | 15 +- .../fflib-extension/caching/NullCache.cls | 71 +- .../{BulkObjectCache.cls => ObjectCache.cls} | 49 +- ....cls-meta.xml => ObjectCache.cls-meta.xml} | 0 .../fflib-extension/caching/OrgCache.cls | 40 +- .../fflib-extension/caching/SessionCache.cls | 22 +- .../fflib-extension/caching/SobjectCache.cls | 139 ---- .../caching/SobjectCache.cls-meta.xml | 5 - .../caching/tests/CachedSoqlExecutorTest.cls | 778 +++++++----------- .../classes/null-objects/NullAction.cls | 2 +- .../classes/null-objects/NullDomain.cls | 2 +- 12 files changed, 389 insertions(+), 759 deletions(-) rename framework/default/ortoo-core/default/classes/fflib-extension/caching/{BulkObjectCache.cls => ObjectCache.cls} (85%) rename framework/default/ortoo-core/default/classes/fflib-extension/caching/{BulkObjectCache.cls-meta.xml => ObjectCache.cls-meta.xml} (100%) delete mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls delete mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls index e6c64ae341a..1f68b28a99e 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/CachedSoqlExecutor.cls @@ -12,7 +12,6 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version - private final static String PARTITION_NAME = 'core'; private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option /** @@ -118,30 +117,8 @@ public inherited sharing class CachedSoqlExecutor //NOPMD: incorrect report of t cacheWrapper.remove( generateKey( soql ) ); } - /** - * Clears the cached results for all cached SOQL - */ - public void clearAllCache() - { - String fullSoqlPartitionName = cacheWrapper.createFullyQualifiedPartitionName( PARTITION_NAME ); - for ( String thisKey : cacheWrapper.getKeys() ) - { - String qualifiedKey = qualifiedKey( thisKey ); - if ( cacheWrapper.contains( qualifiedKey ) ) - { - cacheWrapper.remove( qualifiedKey ); - } - } - } - private String generateKey( String soql ) { - String subkey = EncodingUtil.convertToHex( Crypto.generateDigest( 'SHA1', Blob.valueOf( soql ) ) ); - return qualifiedKey( subkey ); - } - - private String qualifiedKey( String subkey ) - { - return cacheWrapper.createFullyQualifiedKey( PARTITION_NAME, subkey ); + return EncodingUtil.convertToHex( Crypto.generateDigest( 'SHA1', Blob.valueOf( soql ) ) ); } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls index 512d3f5d0bc..75d4a510d9b 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls @@ -9,13 +9,10 @@ */ public interface ICacheAdaptor { - Boolean hasAccessToCache(); - Boolean isACache(); - Object get( String key ); - void put( String key, Object value, Integer lifespan ); - Set getKeys(); - Boolean contains( String key ); - void remove( String key ); - String createFullyQualifiedPartitionName( String partitionName ); - String createFullyQualifiedKey( String partitionName, String key ); + Boolean hasAccessToCache(); + Boolean isACache(); + Object get( String key ); + void put( String key, Object value, Integer lifespan ); + Boolean contains( String key ); + void remove( String key ); } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls index 0007e590817..db5db784508 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls @@ -5,48 +5,33 @@ * * All users are assumed to be allowed to use the cache, but it describes itself as 'not a cache' and effectively does nothing. */ -public class NullCache implements ICacheAdaptor +public inherited sharing class NullCache implements ICacheAdaptor { - public Boolean hasAccessToCache() - { - return true; - } - - public Boolean isACache() - { - return false; - } - - public Object get( String key ) - { - return null; - } - - public void put( String key, Object value, Integer lifespan ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache - { - } - - public Set getKeys() - { - return new Set(); - } - - public Boolean contains( String key ) - { - return false; - } - - public void remove( String key ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache - { - } - - public String createFullyQualifiedPartitionName( String partitionName ) - { - return partitionName; - } - - public String createFullyQualifiedKey( String partitionName, String subKey ) - { - return partitionName + '.' + subkey; - } + public Boolean hasAccessToCache() + { + return true; + } + + public Boolean isACache() + { + return false; + } + + public Object get( String key ) + { + return null; + } + + public void put( String key, Object value, Integer lifespan ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache + { + } + + public Boolean contains( String key ) + { + return false; + } + + public void remove( String key ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache + { + } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls similarity index 85% rename from framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls rename to framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index 4133a2d33a9..c7c4e9dd83b 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -8,7 +8,7 @@ * Note: The lifespan of a given object is reset whenever any object in that object's list is written. * That is, either the whole of the list is aged out, or none of it is. */ -public with sharing class BulkObjectCache +public with sharing class ObjectCache { // TODO: do we need to protect against 100kb limit // TODO: could implement a longest time since accessed, first out @@ -16,8 +16,6 @@ public with sharing class BulkObjectCache // TODO: could just ignore and invalidate the cache - check with the rest of the seniors // TODO: Docs on keys - // TODO: hide createFullyQualifiedPartitionName and createFullyQualifiedKey - public class InvalidIdentifierException extends Exceptions.DeveloperException {} public enum CacheScope { ORG, SESSION } @@ -80,7 +78,6 @@ public with sharing class BulkObjectCache ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version - private final static String PARTITION_NAME = 'core'; private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option /** @@ -89,7 +86,7 @@ public with sharing class BulkObjectCache * @param CacheScope The scope of this cache instance. * @return CachedSoqlExecutor Itself, allowing for a fluent interface */ - public BulkObjectCache setScope( CacheScope scope ) + public ObjectCache setScope( CacheScope scope ) { Contract.requires( scope != null, 'setScope called with a null scope' ); @@ -122,7 +119,7 @@ public with sharing class BulkObjectCache CacheRetrieval values = new CacheRetrieval(); - Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); + Map cachedObjects = (Map)cacheWrapper.get( key ); if ( cachedObjects == null ) { @@ -194,9 +191,9 @@ public with sharing class BulkObjectCache * * @param String The base key to put the objects into * @param List The SObjects to store - * @return BulkObjectCache Itself, allowing for a fluent interface + * @return ObjectCache Itself, allowing for a fluent interface */ - public BulkObjectCache put( String key, List sobjects ) + public ObjectCache put( String key, List sobjects ) { return put( key, 'Id', sobjects ); } @@ -208,9 +205,9 @@ public with sharing class BulkObjectCache * @param String The base key to put the objects into * @param String The field to get the individual SObject identifiers from (field value must be a non-null String) * @param List The SObjects to store - * @return BulkObjectCache Itself, allowing for a fluent interface + * @return ObjectCache Itself, allowing for a fluent interface */ - public BulkObjectCache put( String key, String idField, List sobjects ) + public ObjectCache put( String key, String idField, List sobjects ) { Contract.requires( idField != null, 'put called with a null idField' ); return put( key, new SobjectIdGetter( idField ), sobjects ); @@ -223,16 +220,15 @@ public with sharing class BulkObjectCache * @param String The base key to put the objects into * @param IdGetter The mechanism for getting the Id from each of the given objects * @param List The Objects to store - * @return BulkObjectCache Itself, allowing for a fluent interface + * @return ObjectCache Itself, allowing for a fluent interface */ - public BulkObjectCache put( String key, IdGetter idGetter, List objects ) + public ObjectCache put( String key, IdGetter idGetter, List objects ) { Contract.requires( key != null, 'put called with a null key' ); Contract.requires( idGetter != null, 'put called with a null idGetter' ); Contract.requires( objects != null, 'put called with a null objects' ); - String fullyQualifiedKey = createFullyQualifiedKey( key ); - Map cachedObjects = (Map)cacheWrapper.get( fullyQualifiedKey ); + Map cachedObjects = (Map)cacheWrapper.get( key ); if ( cachedObjects == null ) { @@ -247,7 +243,7 @@ public with sharing class BulkObjectCache Contract.assert( thisId != null, 'put called with an object that has a null Id: ' + thisObject ); cachedObjects.put( thisId, thisObject ); } - cacheWrapper.put( fullyQualifiedKey, cachedObjects, CACHE_LIFESPAN_SECONDS ); + cacheWrapper.put( key, cachedObjects, CACHE_LIFESPAN_SECONDS ); return this; } @@ -256,12 +252,12 @@ public with sharing class BulkObjectCache * Remove all the cached objects from the given base key. * * @param String The base key to remove the objects from. - * @return BulkObjectCache Itself, allowing for a fluent interface + * @return ObjectCache Itself, allowing for a fluent interface */ - public BulkObjectCache remove( String key ) + public ObjectCache remove( String key ) { Contract.requires( key != null, 'remove called with a null key' ); - cacheWrapper.remove( createFullyQualifiedKey( key ) ); + cacheWrapper.remove( key ); return this; } @@ -270,9 +266,9 @@ public with sharing class BulkObjectCache * * @param String The base key to remove the SObjects from. * @param Set The cache identifiers of the SObjects to remove. - * @return BulkObjectCache Itself, allowing for a fluent interface + * @return ObjectCache Itself, allowing for a fluent interface */ - public BulkObjectCache remove( String key, Set ids ) + public ObjectCache remove( String key, Set ids ) { Contract.requires( ids != null, 'remove called with a null ids' ); return remove( key, SetUtils.convertToSetOfStrings( ids ) ); @@ -283,29 +279,24 @@ public with sharing class BulkObjectCache * * @param String The base key to remove the objects from. * @param Set The cache identifiers of the objects to remove. - * @return BulkObjectCache Itself, allowing for a fluent interface + * @return ObjectCache Itself, allowing for a fluent interface */ - public BulkObjectCache remove( String key, Set ids ) + public ObjectCache remove( String key, Set ids ) { Contract.requires( key != null, 'remove called with a null key' ); Contract.requires( ids != null, 'remove called with a null ids' ); - Map cachedObjects = (Map)cacheWrapper.get( createFullyQualifiedKey( key ) ); + Map cachedObjects = (Map)cacheWrapper.get( key ); for ( String thisId : ids ) { Contract.assert( thisId != null, 'remove called with a null entry in ids' ); cachedObjects.remove( thisId ); } - cacheWrapper.put( createFullyQualifiedKey( key ), cachedObjects, CACHE_LIFESPAN_SECONDS ); + cacheWrapper.put( key, cachedObjects, CACHE_LIFESPAN_SECONDS ); return this; } - private String createFullyQualifiedKey( String key ) - { - return cacheWrapper.createFullyQualifiedKey( PARTITION_NAME, key ); - } - private class SobjectIdGetter implements IdGetter { String idField; diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls-meta.xml similarity index 100% rename from framework/default/ortoo-core/default/classes/fflib-extension/caching/BulkObjectCache.cls-meta.xml rename to framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index a4d17b4ba06..acb6eec3422 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -6,11 +6,14 @@ * * Attempting to access the cache without this permission will result in an OrgCache.AccessViolationException */ -public class OrgCache implements ICacheAdaptor +public inherited sharing class OrgCache implements ICacheAdaptor { + // TODO: clear all public class AccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built // in such a way that it's never possible to get this exception + private final static String PARTITION_NAME = 'core'; + private Boolean hasAccessToCache { get @@ -65,7 +68,7 @@ public class OrgCache implements ICacheAdaptor .addContext( 'key', key ); } - return Cache.Org.get( key ); + return Cache.Org.get( createFullyQualifiedKey( key ) ); } /** @@ -93,26 +96,7 @@ public class OrgCache implements ICacheAdaptor .addContext( 'value', value ); } - Cache.Org.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace - } - - /** - * Retrieve a set of the current keys of objects stored in this cache. - * - * If the user does not have access to the cache, will throw an AccessViolationException. - * - * @return Set The keys that currently exist in the cache - */ - public Set getKeys() - { - if ( ! hasAccessToCache ) - { - throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) - .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) - .addContext( 'method', 'getKeys' ); - } - - return Cache.Org.getKeys(); + Cache.Org.put( createFullyQualifiedKey( key ), value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace } /** @@ -132,7 +116,7 @@ public class OrgCache implements ICacheAdaptor .addContext( 'method', 'contains' ) .addContext( 'key', key ); } - return Cache.Org.contains( key ); + return Cache.Org.contains( createFullyQualifiedKey( key ) ); } /** @@ -151,16 +135,16 @@ public class OrgCache implements ICacheAdaptor .addContext( 'method', 'remove' ) .addContext( 'key', key ); } - Cache.Org.remove( key ); + Cache.Org.remove( createFullyQualifiedKey( key ) ); } - public String createFullyQualifiedPartitionName( String partitionName ) + private String createFullyQualifiedPartitionName() { - return Cache.OrgPartition.createFullyQualifiedPartition( PackageUtils.NAMESPACE_PREFIX, partitionName ); + return Cache.OrgPartition.createFullyQualifiedPartition( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME ); } - public String createFullyQualifiedKey( String partitionName, String subKey ) + private String createFullyQualifiedKey( String key ) { - return Cache.OrgPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, partitionName, subkey ); + return Cache.OrgPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, key ); } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls index 4bfc5f4bfec..f65ceb5cd5c 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls @@ -3,8 +3,10 @@ * * All users are assumed to have access to the cache. */ -public class SessionCache implements ICacheAdaptor +public inherited sharing class SessionCache implements ICacheAdaptor { + private static final String PARTITION_NAME = 'core'; + /** * States if this user has access to the cache - which is always true * @@ -49,16 +51,6 @@ public class SessionCache implements ICacheAdaptor Cache.Session.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace } - /** - * Retrieve a set of the current keys of objects stored in this cache. - * - * @return Set The keys that currently exist in the cache - */ - public Set getKeys() - { - return Cache.Session.getKeys(); - } - /** * States if the cache currently contains an object in the given key. * @@ -80,13 +72,13 @@ public class SessionCache implements ICacheAdaptor Cache.Session.remove( key ); } - public String createFullyQualifiedPartitionName( String partitionName ) + private String createFullyQualifiedPartitionName() { - return Cache.SessionPartition.createFullyQualifiedPartition( PackageUtils.NAMESPACE_PREFIX, partitionName ); + return Cache.SessionPartition.createFullyQualifiedPartition( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME ); } - public String createFullyQualifiedKey( String partitionName, String subKey ) + private String createFullyQualifiedKey( String key ) { - return Cache.SessionPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, partitionName, subkey ); + return Cache.SessionPartition.createFullyQualifiedKey( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME, key ); } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls deleted file mode 100644 index 5d2b88601e3..00000000000 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls +++ /dev/null @@ -1,139 +0,0 @@ -// TODO: defunt -public with sharing class SobjectCache -{ - public class CacheAccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built - // in such a way that it's never possible to get this exception - public enum CacheScope { ORG, SESSION } - - ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version - - private final static String PARTITION_NAME = 'soql'; // TODO: same partition? - private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option - - private Boolean hasAccessToCache - { - get - { - if ( hasAccessToCache == null ) - { - hasAccessToCache = PermissionsService.hasAccessToCorePlatformCache(); - } - return hasAccessToCache; - } - set; - } - - public SobjectCache setScope( CacheScope scope ) - { - Contract.requires( scope != null, 'setScope called with a null scope' ); - - switch on scope - { - when ORG - { - cacheWrapper = new OrgCache(); - } - when SESSION - { - cacheWrapper = new SessionCache(); - } - } - - return this; - } - - public CacheRetrieval get( String key, Set ids ) - { - CacheRetrieval values = new CacheRetrieval(); - for ( Id thisId : ids ) - { - SObject thisValue = (SObject)cacheWrapper.get( createFullyQualifiedKey( key, thisId ) ); - if ( thisValue != null ) - { - values.addCacheHit( thisId, thisValue ); - } - else - { - values.addCacheMiss( thisId ); - } - } - return values; - } - - public SobjectCache put( String key, List sobjects ) - { - return put( key, 'Id', sobjects ); - } - - public SobjectCache put( String key, String idField, List sobjects ) - { - for ( Sobject thisSobject : sobjects ) - { - final String thisKey = createFullyQualifiedKey( key, (Id)thisSobject.get( idField ) ); - cacheWrapper.put( thisKey, thisSobject, CACHE_LIFESPAN_SECONDS ); - } - return this; - } - - public SobjectCache remove( String subkeyToRemove ) - { - Set keys = cacheWrapper.getKeys(); - - for ( String thisKey : keys ) - { - if ( isAKeyFor( thisKey, subkeyToRemove ) ) - { - cacheWrapper.remove( thisKey ); - } - } - return this; - } - - public SobjectCache remove( String key, Set ids ) - { - for ( Id thisId : ids ) - { - cacheWrapper.remove( createFullyQualifiedKey( key, thisId ) ); - } - return this; - } - - private Boolean isAKeyFor( String keyToCheck, String keyToCheckAgainst ) - { - return keyToCheck.startsWith( createKeyPrefix( keyToCheckAgainst ) ); - } - - private String createFullyQualifiedKey( String subKey, String id ) - { - return createKeyPrefix( subKey ) + id; - } - - private String createKeyPrefix( String subKey ) - { - return cacheWrapper.createFullyQualifiedKey( PARTITION_NAME, subkey ) + 'x'; - } - - public class CacheRetrieval - { - public Map cacheHits { get; private set; } - public Set cacheMisses { get; private set; } - - private CacheRetrieval() - { - cacheHits = new Map(); - cacheMisses = new Set(); - } - - private CacheRetrieval addCacheMiss( Id id ) - { - cacheMisses.add( id ); - return this; - } - - private CacheRetrieval addCacheHit( Id id, Sobject value ) - { - cacheHits.put( id, value ); - return this; - } - } -} diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml deleted file mode 100644 index dd61d1f917e..00000000000 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SobjectCache.cls-meta.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - 52.0 - Active - diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls index 1ead66f5e98..f54244a8e4f 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls @@ -1,581 +1,429 @@ @isTest private without sharing class CachedSoqlExecutorTest { - @isTest - private static void query_whenCalledTwiceByAUserWithAccessToTheCache_onlyIssuesOneSoqlStatement() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void query_whenCalledTwiceByAUserWithAccessToTheCache_onlyIssuesOneSoqlStatement() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( true ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + CachedSoqlExecutor executor = new CachedSoqlExecutor(); - Test.startTest(); - List originalResults = executor.query( soqlStatement ); - List secondResults = executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + Test.startTest(); + List originalResults = executor.query( soqlStatement ); + List secondResults = executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with access to the cache, will only issue one SOQL statement' ); - System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with access to the cache, returns the same results in both calls' ); - } + System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with access to the cache, will only issue one SOQL statement' ); + System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with access to the cache, returns the same results in both calls' ); + } - @isTest - private static void query_org_whenCalledTwiceByAUserWithAccessToTheCache_onlyIssuesOneSoqlStatement() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void query_org_whenCalledTwiceByAUserWithAccessToTheCache_onlyIssuesOneSoqlStatement() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( true ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - Test.startTest(); - List originalResults = executor.query( soqlStatement ); - List secondResults = executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + Test.startTest(); + List originalResults = executor.query( soqlStatement ); + List secondResults = executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with access to the cache, will only issue one SOQL statement' ); - System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with access to the cache, returns the same results in both calls' ); - } + System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with access to the cache, will only issue one SOQL statement' ); + System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with access to the cache, returns the same results in both calls' ); + } - @isTest - private static void query_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesTwoSoqlStatements() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void query_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesTwoSoqlStatements() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( false ); + setupAccessToSoqlCache( false ); - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + CachedSoqlExecutor executor = new CachedSoqlExecutor(); - Test.startTest(); - List originalResults = executor.query( soqlStatement ); - List secondResults = executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + Test.startTest(); + List originalResults = executor.query( soqlStatement ); + List secondResults = executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - System.assertEquals( 2, soqlCalls, 'query, when called twice by a user with no access to the cache, will issue two SOQL statements' ); - System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with not access to the cache, returns the same results in both calls' ); - } + System.assertEquals( 2, soqlCalls, 'query, when called twice by a user with no access to the cache, will issue two SOQL statements' ); + System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with not access to the cache, returns the same results in both calls' ); + } - @isTest - private static void clearAllCache_willClearAllStatementsAndResultsFromTheCache() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void clearCacheFor_whenGivenASoqlStatementThatHasBeenExecuted_willClearTheCacheForThatStatement() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( true ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + CachedSoqlExecutor executor = new CachedSoqlExecutor(); - Test.startTest(); + executor.query( soqlStatement ); - executor.query( soqlStatement ); // executes SOQL - executor.clearAllCache(); + Test.startTest(); + executor.clearCacheFor( soqlStatement ); + executor.query( soqlStatement ); // should execute another soql + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - executor.query( soqlStatement ); // executes another SOQL - executor.query( soqlStatement ); - executor.query( soqlStatement ); + System.assertEquals( 1, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will clear that soql from the cache' ); + } + @isTest + private static void clearCacheFor_whenGivenASoqlStatementThatHasBeenExecuted_willNotClearTheCacheForOtherStatements() // NOPMD: Test method name format + { + String soqlStatement1 = 'SELECT Id FROM Account'; + String soqlStatement2 = 'SELECT Id FROM Account LIMIT 1'; - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + setupAccessToSoqlCache( true ); - System.assertEquals( 2, soqlCalls, 'clearAllCache, when called, will clear statements and results from the cache meaning that SOQL executions will need to be repeated when query is called' ); - } + CachedSoqlExecutor executor = new CachedSoqlExecutor(); - @isTest - private static void clearAllCache_whenThereIsNothingInTheCache_willNotThrowAnException() // NOPMD: Test method name format - { - setupAccessToSoqlCache( true ); + executor.query( soqlStatement1 ); + executor.query( soqlStatement2 ); - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + Test.startTest(); + executor.clearCacheFor( soqlStatement1 ); + executor.query( soqlStatement2 ); // should not execute another soql + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - Test.startTest(); - executor.clearAllCache(); - Test.stopTest(); + System.assertEquals( 0, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will not clear other soql from the cache' ); + } - System.assert( true, 'clearAllCache, when there is nothing in the cache, will not throw an exception' ); - } + @isTest + private static void clearCacheFor_whenGivenASoqlStatementThatHasNotBeenExecuted_willNotThrowAnException() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - @isTest - private static void clearAllCache_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format - { - setupAccessToSoqlCache( false ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + CachedSoqlExecutor executor = new CachedSoqlExecutor(); - Test.startTest(); - Exception exceptionThrown; - try - { - executor.clearAllCache(); - } - catch ( OrgCache.AccessViolationException e ) - { - exceptionThrown = e; - } - Test.stopTest(); + Test.startTest(); + executor.clearCacheFor( soqlStatement ); + Test.stopTest(); - ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'clearAllCache, when the user does not have access to the cache, will throw an exception' ); - } + System.assert( true, 'clearCacheFor, when given a SOQL statement that has not been executed, will not throw an exception' ); + } - @isTest - private static void clearCacheFor_whenGivenASoqlStatementThatHasBeenExecuted_willClearTheCacheForThatStatement() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void clearCacheFor_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( true ); + setupAccessToSoqlCache( false ); - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + CachedSoqlExecutor executor = new CachedSoqlExecutor(); - executor.query( soqlStatement ); + Test.startTest(); + Exception exceptionThrown; + try + { + executor.clearCacheFor( soqlStatement ); + } + catch ( OrgCache.AccessViolationException e ) + { + exceptionThrown = e; + } + Test.stopTest(); - Test.startTest(); - executor.clearCacheFor( soqlStatement ); - executor.query( soqlStatement ); // should execute another soql - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'clearCacheFor, when the user does not have access to the cache, will throw an exception' ); + } - System.assertEquals( 1, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will clear that soql from the cache' ); - } + @isTest + private static void query_whenRanFor100Queries_willNotThrowAnException() + { + List soqlStatements = new List(); + for ( Integer i=1; i<=100; i++ ) + { + soqlStatements.add( 'SELECT Id FROM Account LIMIT ' + i ); + } - @isTest - private static void clearCacheFor_whenGivenASoqlStatementThatHasBeenExecuted_willNotClearTheCacheForOtherStatements() // NOPMD: Test method name format - { - String soqlStatement1 = 'SELECT Id FROM Account'; - String soqlStatement2 = 'SELECT Id FROM Account LIMIT 1'; + setupAccessToSoqlCache( true ); - setupAccessToSoqlCache( true ); + CachedSoqlExecutor executor = new CachedSoqlExecutor(); - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + Test.startTest(); - executor.query( soqlStatement1 ); - executor.query( soqlStatement2 ); + // Run each statement multiple times, one by one + for ( String thisSoqlStatement : soqlStatements ) + { + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + } - Test.startTest(); - executor.clearCacheFor( soqlStatement1 ); - executor.query( soqlStatement2 ); // should not execute another soql - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + // Then run each statement again + for ( String thisSoqlStatement : soqlStatements ) + { + executor.query( thisSoqlStatement ); + } - System.assertEquals( 0, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will not clear other soql from the cache' ); - } + Test.stopTest(); - @isTest - private static void clearCacheFor_whenGivenASoqlStatementThatHasNotBeenExecuted_willNotThrowAnException() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + System.assert( true, 'query, when run multiple times for 100 distinct queries, will not throw an exception' ); + } - setupAccessToSoqlCache( true ); + @isTest + private static void query_session_whenCalledTwiceByAUserWithAccessToTheCache_onlyIssuesOneSoqlStatement() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + setupAccessToSoqlCache( true ); - Test.startTest(); - executor.clearCacheFor( soqlStatement ); - Test.stopTest(); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - System.assert( true, 'clearCacheFor, when given a SOQL statement that has not been executed, will not throw an exception' ); - } + Test.startTest(); + List originalResults = executor.query( soqlStatement ); + List secondResults = executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - @isTest - private static void clearCacheFor_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with access to the cache, will only issue one SOQL statement' ); + System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with access to the cache, returns the same results in both calls' ); + } - setupAccessToSoqlCache( false ); + @isTest + private static void query_session_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesOneSoqlStatement() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + setupAccessToSoqlCache( false ); - Test.startTest(); - Exception exceptionThrown; - try - { - executor.clearCacheFor( soqlStatement ); - } - catch ( OrgCache.AccessViolationException e ) - { - exceptionThrown = e; - } - Test.stopTest(); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'clearCacheFor, when the user does not have access to the cache, will throw an exception' ); - } + Test.startTest(); + List originalResults = executor.query( soqlStatement ); + List secondResults = executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - @isTest - private static void query_whenRanFor100Queries_willNotThrowAnException() - { - List soqlStatements = new List(); - for ( Integer i=1; i<=100; i++ ) - { - soqlStatements.add( 'SELECT Id FROM Account LIMIT ' + i ); - } + System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with no access to the org cache, will issue one SOQL statement' ); + System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with no access to the cache, returns the same results in both calls' ); + } - setupAccessToSoqlCache( true ); + @isTest + private static void clearCacheFor_session_whenGivenASoqlStatementThatHasBeenExecuted_willClearTheCacheForThatStatement() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - CachedSoqlExecutor executor = new CachedSoqlExecutor(); + setupAccessToSoqlCache( true ); - Test.startTest(); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - // Run each statement multiple times, one by one - for ( String thisSoqlStatement : soqlStatements ) - { - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - } + executor.query( soqlStatement ); - // Then run each statement again - for ( String thisSoqlStatement : soqlStatements ) - { - executor.query( thisSoqlStatement ); - } + Test.startTest(); + executor.clearCacheFor( soqlStatement ); + executor.query( soqlStatement ); // should execute another soql + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - Test.stopTest(); + System.assertEquals( 1, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will clear that soql from the cache' ); + } - System.assert( true, 'query, when run multiple times for 100 distinct queries, will not throw an exception' ); - } + @isTest + private static void clearCacheFor_session_whenGivenASoqlStatementThatHasBeenExecuted_willNotClearTheCacheForOtherStatements() // NOPMD: Test method name format + { + String soqlStatement1 = 'SELECT Id FROM Account'; + String soqlStatement2 = 'SELECT Id FROM Account LIMIT 1'; - @isTest - private static void query_session_whenCalledTwiceByAUserWithAccessToTheCache_onlyIssuesOneSoqlStatement() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + setupAccessToSoqlCache( true ); - setupAccessToSoqlCache( true ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); + executor.query( soqlStatement1 ); + executor.query( soqlStatement2 ); - Test.startTest(); - List originalResults = executor.query( soqlStatement ); - List secondResults = executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + Test.startTest(); + executor.clearCacheFor( soqlStatement1 ); + executor.query( soqlStatement2 ); // should not execute another soql + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with access to the cache, will only issue one SOQL statement' ); - System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with access to the cache, returns the same results in both calls' ); - } + System.assertEquals( 0, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will not clear other soql from the cache' ); + } - @isTest - private static void query_session_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesOneSoqlStatement() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void clearCacheFor_session_whenGivenASoqlStatementThatHasNotBeenExecuted_willNotThrowAnException() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( false ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - Test.startTest(); - List originalResults = executor.query( soqlStatement ); - List secondResults = executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + Test.startTest(); + executor.clearCacheFor( soqlStatement ); + Test.stopTest(); - System.assertEquals( 1, soqlCalls, 'query, when called twice by a user with no access to the org cache, will issue one SOQL statement' ); - System.assertEquals( originalResults, secondResults, 'query, when called twice by a user with no access to the cache, returns the same results in both calls' ); - } + System.assert( true, 'clearCacheFor, when given a SOQL statement that has not been executed, will not throw an exception' ); + } - @isTest - private static void clearAllCache_session_willClearAllStatementsAndResultsFromTheCache() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void clearCacheFor_session_whenTheUserDoesNotHaveAccessToTheOrgCache_willNotThrowAnException() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( true ); + setupAccessToSoqlCache( false ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - Test.startTest(); + Test.startTest(); + executor.clearCacheFor( soqlStatement ); + Test.stopTest(); - executor.query( soqlStatement ); // executes SOQL - executor.clearAllCache(); - - executor.query( soqlStatement ); // executes another SOQL - executor.query( soqlStatement ); - executor.query( soqlStatement ); - - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); - - System.assertEquals( 2, soqlCalls, 'clearAllCache, when called, will clear statements and results from the cache meaning that SOQL executions will need to be repeated when query is called' ); - } - - @isTest - private static void clearAllCache_session_whenThereIsNothingInTheCache_willNotThrowAnException() // NOPMD: Test method name format - { - setupAccessToSoqlCache( true ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - - Test.startTest(); - executor.clearAllCache(); - Test.stopTest(); - - System.assert( true, 'clearAllCache, when there is nothing in the cache, will not throw an exception' ); - } - - @isTest - private static void clearAllCache_session_whenTheUserDoesNotHaveAccessToTheOrgCache_willNotThrowAnException() // NOPMD: Test method name format - { - setupAccessToSoqlCache( false ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - - Test.startTest(); - executor.clearAllCache(); - Test.stopTest(); - - System.assert( true, 'clearAllCache, when the user does not have access to the org cache, will not throw an exception' ); - } - - @isTest - private static void clearCacheFor_session_whenGivenASoqlStatementThatHasBeenExecuted_willClearTheCacheForThatStatement() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; - - setupAccessToSoqlCache( true ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - - executor.query( soqlStatement ); - - Test.startTest(); - executor.clearCacheFor( soqlStatement ); - executor.query( soqlStatement ); // should execute another soql - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); - - System.assertEquals( 1, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will clear that soql from the cache' ); - } - - @isTest - private static void clearCacheFor_session_whenGivenASoqlStatementThatHasBeenExecuted_willNotClearTheCacheForOtherStatements() // NOPMD: Test method name format - { - String soqlStatement1 = 'SELECT Id FROM Account'; - String soqlStatement2 = 'SELECT Id FROM Account LIMIT 1'; - - setupAccessToSoqlCache( true ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - - executor.query( soqlStatement1 ); - executor.query( soqlStatement2 ); - - Test.startTest(); - executor.clearCacheFor( soqlStatement1 ); - executor.query( soqlStatement2 ); // should not execute another soql - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); - - System.assertEquals( 0, soqlCalls, 'clearCacheFor, when given a SOQL statement that is already in the cache, will not clear other soql from the cache' ); - } - - @isTest - private static void clearCacheFor_session_whenGivenASoqlStatementThatHasNotBeenExecuted_willNotThrowAnException() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; - - setupAccessToSoqlCache( true ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - - Test.startTest(); - executor.clearCacheFor( soqlStatement ); - Test.stopTest(); - - System.assert( true, 'clearCacheFor, when given a SOQL statement that has not been executed, will not throw an exception' ); - } - - @isTest - private static void clearCacheFor_session_whenTheUserDoesNotHaveAccessToTheOrgCache_willNotThrowAnException() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; - - setupAccessToSoqlCache( false ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - - Test.startTest(); - executor.clearCacheFor( soqlStatement ); - Test.stopTest(); - - System.assert( true, 'clearCacheFor, when the user does not have access to the org cache, will not throw an exception' ); + System.assert( true, 'clearCacheFor, when the user does not have access to the org cache, will not throw an exception' ); } - @isTest - private static void query_session_whenRanFor100Queries_willNotThrowAnException() - { - List soqlStatements = new List(); - for ( Integer i=1; i<=100; i++ ) - { - soqlStatements.add( 'SELECT Id FROM Account LIMIT ' + i ); - } - - setupAccessToSoqlCache( true ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - - Test.startTest(); - - // Run each statement multiple times, one by one - for ( String thisSoqlStatement : soqlStatements ) - { - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - executor.query( thisSoqlStatement ); - } - - // Then run each statement again - for ( String thisSoqlStatement : soqlStatements ) - { - executor.query( thisSoqlStatement ); - } - - Test.stopTest(); - - System.assert( true, 'query, when run multiple times for 100 distinct queries, will not throw an exception' ); - } - - @isTest - private static void query_none_whenCalledTwiceByAUserWithAccessToTheCache_issuesTwoSoqlStatements() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; - - setupAccessToSoqlCache( true ); - - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); - - Test.startTest(); - List originalResults = executor.query( soqlStatement ); - List secondResults = executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); - - System.assertEquals( 2, soqlCalls, 'query against a NONE cache, when called twice by a user with access to the cache, will issue two SOQL statements' ); - } - - @isTest - private static void query_none_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesTwoSoqlStatements() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void query_session_whenRanFor100Queries_willNotThrowAnException() + { + List soqlStatements = new List(); + for ( Integer i=1; i<=100; i++ ) + { + soqlStatements.add( 'SELECT Id FROM Account LIMIT ' + i ); + } - setupAccessToSoqlCache( false ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); - Test.startTest(); - List originalResults = executor.query( soqlStatement ); - List secondResults = executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + Test.startTest(); - System.assertEquals( 2, soqlCalls, 'query against a NONE cache, when called twice by a user with no access to the cache, will issue two SOQL statements' ); - } + // Run each statement multiple times, one by one + for ( String thisSoqlStatement : soqlStatements ) + { + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + executor.query( thisSoqlStatement ); + } - @isTest - private static void clearAllCache_none_willNotThrowAnException() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + // Then run each statement again + for ( String thisSoqlStatement : soqlStatements ) + { + executor.query( thisSoqlStatement ); + } - setupAccessToSoqlCache( true ); + Test.stopTest(); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); + System.assert( true, 'query, when run multiple times for 100 distinct queries, will not throw an exception' ); + } - Test.startTest(); + @isTest + private static void query_none_whenCalledTwiceByAUserWithAccessToTheCache_issuesTwoSoqlStatements() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - executor.query( soqlStatement ); - executor.clearAllCache(); + setupAccessToSoqlCache( true ); - executor.query( soqlStatement ); - executor.query( soqlStatement ); - executor.query( soqlStatement ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); + Test.startTest(); + List originalResults = executor.query( soqlStatement ); + List secondResults = executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + System.assertEquals( 2, soqlCalls, 'query against a NONE cache, when called twice by a user with access to the cache, will issue two SOQL statements' ); + } - System.assertEquals( 4, soqlCalls, 'clearAllCache against a none cache, when called, will not throw an exception and will not affect the number of SOQL statements issued' ); - } + @isTest + private static void query_none_whenCalledTwiceByAUserWithoutAccessToTheCache_issuesTwoSoqlStatements() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - @isTest - private static void clearAllCache_none_whenTheUserDoesNotHaveAccessToTheOrgCache_willNotThrowAnException() // NOPMD: Test method name format - { - setupAccessToSoqlCache( false ); + setupAccessToSoqlCache( false ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); - Test.startTest(); - executor.clearAllCache(); - Test.stopTest(); + Test.startTest(); + List originalResults = executor.query( soqlStatement ); + List secondResults = executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - System.assert( true, 'clearAllCache against a none cache, when called by a user that does not have access to the org cache, will not throw an exception' ); - } + System.assertEquals( 2, soqlCalls, 'query against a NONE cache, when called twice by a user with no access to the cache, will issue two SOQL statements' ); + } - @isTest - private static void clearCacheFor_none_doesNotAffectTheNumberOfSoqlStatementsIssued() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void clearCacheFor_none_doesNotAffectTheNumberOfSoqlStatementsIssued() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( true ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); - executor.query( soqlStatement ); + executor.query( soqlStatement ); - Test.startTest(); - executor.clearCacheFor( soqlStatement ); - executor.query( soqlStatement ); - Integer soqlCalls = Limits.getQueries(); - Test.stopTest(); + Test.startTest(); + executor.clearCacheFor( soqlStatement ); + executor.query( soqlStatement ); + Integer soqlCalls = Limits.getQueries(); + Test.stopTest(); - System.assertEquals( 1, soqlCalls, 'clearCacheFor against a none cache, does not affect the number of SOQL statements issued' ); - } + System.assertEquals( 1, soqlCalls, 'clearCacheFor against a none cache, does not affect the number of SOQL statements issued' ); + } - @isTest - private static void clearCacheFor_none_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format - { - String soqlStatement = 'SELECT Id FROM Account'; + @isTest + private static void clearCacheFor_none_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + String soqlStatement = 'SELECT Id FROM Account'; - setupAccessToSoqlCache( false ); + setupAccessToSoqlCache( false ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); - Test.startTest(); - executor.clearCacheFor( soqlStatement ); - Test.stopTest(); + Test.startTest(); + executor.clearCacheFor( soqlStatement ); + Test.stopTest(); - System.assert( true, 'clearCacheFor against a none cache, when called by a user that does not have access to the org cache, will not throw an exception' ); - } + System.assert( true, 'clearCacheFor against a none cache, when called by a user that does not have access to the org cache, will not throw an exception' ); + } - @isTest - private static void query_none_whenRanFor100Queries_willNotThrowAnException() - { - List soqlStatements = new List(); - for ( Integer i=1; i<=100; i++ ) - { - soqlStatements.add( 'SELECT Id FROM Account LIMIT ' + i ); - } + @isTest + private static void query_none_whenRanFor100Queries_willNotThrowAnException() + { + List soqlStatements = new List(); + for ( Integer i=1; i<=100; i++ ) + { + soqlStatements.add( 'SELECT Id FROM Account LIMIT ' + i ); + } - setupAccessToSoqlCache( true ); + setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.NONE ); - Test.startTest(); - // Run each statement once - this is the maximum we can do - for ( String thisSoqlStatement : soqlStatements ) - { - executor.query( thisSoqlStatement ); - } - Test.stopTest(); + Test.startTest(); + // Run each statement once - this is the maximum we can do + for ( String thisSoqlStatement : soqlStatements ) + { + executor.query( thisSoqlStatement ); + } + Test.stopTest(); - System.assert( true, 'query against a none cache, when run for 100 queries, will not throw an exception' ); - } + System.assert( true, 'query against a none cache, when run for 100 queries, will not throw an exception' ); + } - private static void setupAccessToSoqlCache( Boolean accessToCache ) - { - ApplicationMockRegistrar.registerMockService( IPermissionsService.class ) - .when( 'hasAccessToCorePlatformCache' ) - .returns( accessToCache ); - } + private static void setupAccessToSoqlCache( Boolean accessToCache ) + { + ApplicationMockRegistrar.registerMockService( IPermissionsService.class ) + .when( 'hasAccessToCorePlatformCache' ) + .returns( accessToCache ); + } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/null-objects/NullAction.cls b/framework/default/ortoo-core/default/classes/null-objects/NullAction.cls index 233f6022f00..2fbc48b8fb0 100644 --- a/framework/default/ortoo-core/default/classes/null-objects/NullAction.cls +++ b/framework/default/ortoo-core/default/classes/null-objects/NullAction.cls @@ -1,7 +1,7 @@ /** * Provides the ability to perform no action in a Process Builder built process */ -public class NullAction +public inherited sharing class NullAction { /** * Allows a Process Builder or Flow action to 'Do Nothing' diff --git a/framework/default/ortoo-core/default/classes/null-objects/NullDomain.cls b/framework/default/ortoo-core/default/classes/null-objects/NullDomain.cls index 9a1a1b3998b..e6b29237397 100644 --- a/framework/default/ortoo-core/default/classes/null-objects/NullDomain.cls +++ b/framework/default/ortoo-core/default/classes/null-objects/NullDomain.cls @@ -1,7 +1,7 @@ /** * Is a Domain object that does nothing other than track that certain overridden methods have been called */ -public class NullDomain extends ortoo_SobjectDomain +public inherited sharing class NullDomain extends ortoo_SobjectDomain { public static Set methodsCalled = new Set(); From c60ecc3d682299fdb693f1214717ef22c7f0c19d Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 14:40:28 +0000 Subject: [PATCH 11/28] Added removeAll to the OrgCache and fixed test for setting ORG scope --- .../fflib-extension/caching/OrgCache.cls | 23 ++++++++++++++++++- .../caching/tests/CachedSoqlExecutorTest.cls | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index acb6eec3422..00c55e7548d 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -8,7 +8,6 @@ */ public inherited sharing class OrgCache implements ICacheAdaptor { - // TODO: clear all public class AccessViolationException extends Exceptions.DeveloperException {} // this looks like a config exception, but actually the system should be built // in such a way that it's never possible to get this exception @@ -138,6 +137,28 @@ public inherited sharing class OrgCache implements ICacheAdaptor Cache.Org.remove( createFullyQualifiedKey( key ) ); } + /** + * Instucts the cache to remove all objects from the cache. + * + * Is useful for clearing the cache out completely when the majority of entries would otherwise be dirty. + * + * If the user does not have access to the cache, will throw an AccessViolationException. + */ + public void removeAll() + { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'removeAll' ); + } + + for ( String thisKey : Cache.Org.getKeys() ) + { + Cache.Org.remove( thisKey ); + } + } + private String createFullyQualifiedPartitionName() { return Cache.OrgPartition.createFullyQualifiedPartition( PackageUtils.NAMESPACE_PREFIX, PARTITION_NAME ); diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls index f54244a8e4f..32fecce5870 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/CachedSoqlExecutorTest.cls @@ -27,7 +27,7 @@ private without sharing class CachedSoqlExecutorTest setupAccessToSoqlCache( true ); - CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.SESSION ); + CachedSoqlExecutor executor = new CachedSoqlExecutor().setScope( CachedSoqlExecutor.CacheScope.ORG ); Test.startTest(); List originalResults = executor.query( soqlStatement ); From 3906e1a2e49b01aacf04dcb940180c6957ee986b Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 14:45:56 +0000 Subject: [PATCH 12/28] Added a test for NullCache --- .../fflib-extension/caching/NullCache.cls | 2 +- .../caching/tests/NullCacheTest.cls | 45 +++++++++++++++++++ .../caching/tests/NullCacheTest.cls-meta.xml | 5 +++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls index db5db784508..8bac52b4ab9 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls @@ -1,5 +1,5 @@ /** - * An implementation of the ICacheAdaptor that is benign. + * An implementation of the ICacheAdaptor that is benign - conforms to the Null Object Pattern * * Allows for the use of code that will automatically interact with a cache and be able to switch that off dynamically and simply. * diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls new file mode 100644 index 00000000000..a4c62694f59 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls @@ -0,0 +1,45 @@ +@isTest +private without sharing class NullCacheTest +{ + @isTest + private static void hasAccessToCache_whenCalled_returnsTrue() // NOPMD: Test method name format + { + Boolean got = new NullCache().hasAccessToCache(); + System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); + } + + @isTest + private static void isACache_whenCalled_returnsFalse() // NOPMD: Test method name format + { + Boolean got = new NullCache().isACache(); + System.assertEquals( false, got, 'hasAccessToCache, when called, will return false' ); + } + + @isTest + private static void get_whenCalled_returnsNull() // NOPMD: Test method name format + { + Object got = new NullCache().get( null ); + System.assertEquals( null, got, 'get, when called, will return null' ); + } + + @isTest + private static void put_whenCalled_doesNothing() // NOPMD: Test method name format + { + new NullCache().put( null, null, null ); + System.assert( true, 'put, when called, will do nothing and not throw an exception' ); + } + + @isTest + private static void contains_whenCalled_returnsFalse() // NOPMD: Test method name format + { + Boolean got = new NullCache().contains( null ); + System.assertEquals( false, got, 'contains, when called, will return false' ); + } + + @isTest + private static void remove_whenCalled_doesNothing() // NOPMD: Test method name format + { + new NullCache().remove( null ); + System.assert( true, 'remove, when called, will do nothing and not throw an exception' ); + } +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + From 525aa2ced230aa9158f2359f1384f2390a7a66c2 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 15:14:09 +0000 Subject: [PATCH 13/28] Added test and robustness to SessionCache Fixed missing assertion text from NullDomainTest --- .../fflib-extension/caching/NullCache.cls | 2 +- .../fflib-extension/caching/SessionCache.cls | 11 +++ .../caching/tests/SessionCacheTest.cls | 98 +++++++++++++++++++ .../tests/SessionCacheTest.cls-meta.xml | 5 + .../null-objects/tests/NullDomainTest.cls | 2 +- 5 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls index 8bac52b4ab9..c2bf10eb631 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls @@ -1,5 +1,5 @@ /** - * An implementation of the ICacheAdaptor that is benign - conforms to the Null Object Pattern + * An implementation of the ICacheAdaptor that is benign - conforms to the Null Object Pattern. * * Allows for the use of code that will automatically interact with a cache and be able to switch that off dynamically and simply. * diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls index f65ceb5cd5c..90c8441d770 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls @@ -35,6 +35,7 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public Object get( String key ) { + Contract.requires( key != null, 'get called with a null key' ); return Cache.Session.get( key ); } @@ -48,6 +49,12 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public void put( String key, Object value, Integer lifespan ) { + Contract.requires( key != null, 'put called with a null key' ); + Contract.requires( value != null, 'put called with a null value (call remove instead)' ); + Contract.requires( lifespan != null, 'put called with a null lifespan' ); + Contract.requires( lifespan >= 0, 'put called with a negative lifespan' ); + // Note that the maximum is handled by Salesforce, just in case it increases + Cache.Session.put( key, value, lifespan, Cache.Visibility.NAMESPACE, true ); // immutable outside of namespace } @@ -59,6 +66,8 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public Boolean contains( String key ) { + Contract.requires( key != null, 'contains called with a null key' ); + return Cache.Session.contains( key ); } @@ -69,6 +78,8 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public void remove( String key ) { + Contract.requires( key != null, 'remove called with a null key' ); + Cache.Session.remove( key ); } diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls new file mode 100644 index 00000000000..5bcdfb4067e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls @@ -0,0 +1,98 @@ +@isTest +private without sharing class SessionCacheTest +{ + private final static Integer DEFAULT_LIFESPAN = 1000; + + @isTest + private static void hasAccessToCache_whenCalled_returnsTrue() // NOPMD: Test method name format + { + Boolean got = new SessionCache().hasAccessToCache(); + System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); + } + + @isTest + private static void isACache_whenCalled_returnsTrue() // NOPMD: Test method name format + { + Boolean got = new SessionCache().isACache(); + System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); + } + + @isTest + private static void get_whenCalledWithAKeyNotInTheCache_returnsNull() // NOPMD: Test method name format + { + Object got = new SessionCache().get( 'doesnotexist' ); + System.assertEquals( null, got, 'get, when called with a key that is not in the cache, will return null' ); + } + + @isTest + private static void get_whenCalledWithAKeyThatWasPut_returnsIt() // NOPMD: Test method name format + { + String expected = 'thecachedthing'; + new SessionCache().put( 'key', expected, DEFAULT_LIFESPAN ); + + Object got = new SessionCache().get( 'key' ); + System.assertEquals( expected, got, 'get, when called with a key that is in the cache, will return it' ); + } + + @isTest + private static void put_whenCalledMultipleTimesWithTheSameKey_overwritesIt() // NOPMD: Test method name format + { + new SessionCache().put( 'key', '1', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key', '2', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key', '3', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key', '4', DEFAULT_LIFESPAN ); + + Object got = new SessionCache().get( 'key' ); + System.assertEquals( '4', got, 'put, when called multiple times with the same key, overwrites it' ); + } + + @isTest + private static void put_whenCalledMultipleTimesWithDifferentKeys_storesThem() // NOPMD: Test method name format + { + new SessionCache().put( 'key1', '1', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key2', '2', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key3', '3', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key4', '4', DEFAULT_LIFESPAN ); + + Object got = new SessionCache().get( 'key2' ); + System.assertEquals( '2', got, 'put, when called multiple times with different keys, stores them' ); + } + + @isTest + private static void contains_whenCalledForAKeyThatWasPut_returnsTrue() // NOPMD: Test method name format + { + new SessionCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); + + Boolean got = new SessionCache().contains( 'keythatexists' ); + System.assertEquals( true, got, 'contains, when called for a key that was put, returns true' ); + } + + @isTest + private static void contains_whenCalledForAKeyThatWasNotPut_returnsFalse() // NOPMD: Test method name format + { + new SessionCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); + + Boolean got = new SessionCache().contains( 'keythatdoesnotexist' ); + System.assertEquals( false, got, 'contains, when called for a key that was not put, returns false' ); + } + + @isTest + private static void remove_whenCalledForAKeyThatWasPut_removesIt() // NOPMD: Test method name format + { + new SessionCache().put( 'originalkey', '1', DEFAULT_LIFESPAN ); + + new SessionCache().remove( 'originalkey' ); + + System.assertEquals( false, new SessionCache().contains( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking contains' ); + System.assertEquals( null, new SessionCache().get( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking get' ); + } + + @isTest + private static void remove_whenCalledForAKeyThatWasNotPut_doesNotError() // NOPMD: Test method name format + { + new SessionCache().remove( 'keythatneverexisted' ); + + System.assertEquals( false, new SessionCache().contains( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking contains' ); + System.assertEquals( null, new SessionCache().get( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking get' ); + } +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + diff --git a/framework/default/ortoo-core/default/classes/null-objects/tests/NullDomainTest.cls b/framework/default/ortoo-core/default/classes/null-objects/tests/NullDomainTest.cls index 1740494c825..d57a00b9f34 100644 --- a/framework/default/ortoo-core/default/classes/null-objects/tests/NullDomainTest.cls +++ b/framework/default/ortoo-core/default/classes/null-objects/tests/NullDomainTest.cls @@ -7,7 +7,7 @@ private without sharing class NullDomainTest private static void constructorClass_buildsANullDomain() // NOPMD: Test method name format { NullDomain nullDomain = (NullDomain)new NullDomain.Constructor().construct( LIST_OF_SOBJECTS ); - System.assertNotEquals( null, nullDomain ); + System.assertNotEquals( null, nullDomain, 'constructor, when called, does not throw an exception' ); } @isTest From 86f84bc2d577d6408cf3e590eb4ddd147d1c5cf3 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 15:30:11 +0000 Subject: [PATCH 14/28] Added tests for the OrgCache Added more contracts for robustness --- .../fflib-extension/caching/OrgCache.cls | 5 + .../caching/tests/OrgCacheTest.cls | 276 ++++++++++++++++++ .../caching/tests/OrgCacheTest.cls-meta.xml | 5 + 3 files changed, 286 insertions(+) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index 00c55e7548d..a8445d987a7 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -84,6 +84,7 @@ public inherited sharing class OrgCache implements ICacheAdaptor { Contract.requires( key != null, 'put called with a null key' ); Contract.requires( lifespan != null, 'put called with a null lifespan' ); + Contract.requires( value != null, 'put called with a null value' ); Contract.requires( lifespan >= 0, 'put called with a negaitve lifespan' ); if ( ! hasAccessToCache ) @@ -108,6 +109,8 @@ public inherited sharing class OrgCache implements ICacheAdaptor */ public Boolean contains( String key ) { + Contract.requires( key != null, 'contains called with a null key' ); + if ( ! hasAccessToCache ) { throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) @@ -127,6 +130,8 @@ public inherited sharing class OrgCache implements ICacheAdaptor */ public void remove( String key ) { + Contract.requires( key != null, 'remove called with a null key' ); + if ( ! hasAccessToCache ) { throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls new file mode 100644 index 00000000000..113169b1207 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls @@ -0,0 +1,276 @@ +@isTest +private without sharing class OrgCacheTest +{ + private final static Integer DEFAULT_LIFESPAN = 1000; + + @isTest + private static void hasAccessToCache_whenCalledByUserWithCacheAccess_returnsTrue() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + Boolean got = new OrgCache().hasAccessToCache(); + System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); + } + + @isTest + private static void hasAccessToCache_whenCalledByUserWithoutCacheAccess_returnsFalse() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + Boolean got = new OrgCache().hasAccessToCache(); + System.assertEquals( false, got, 'hasAccessToCache, when called, will return false' ); + } + + @isTest + private static void isACache_whenCalledByUserWithCacheAccess_returnsTrue() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + Boolean got = new OrgCache().isACache(); + System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); + } + + @isTest + private static void isACache_whenCalledByUserWithoutCacheAccess_returnsTrue() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + Boolean got = new OrgCache().isACache(); + System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); + } + + @isTest + private static void get_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + OrgCache cache = new OrgCache(); + + Test.startTest(); + Exception exceptionThrown; + try + { + cache.get( 'a key' ); + } + catch ( OrgCache.AccessViolationException e ) + { + exceptionThrown = e; + } + Test.stopTest(); + + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'get, when the user does not have access to the cache, will throw an exception' ); + } + + @isTest + private static void get_whenCalledByUserWithCacheAccessWithAKeyNotInTheCache_returnsNull() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + Object got = new OrgCache().get( 'doesnotexist' ); + System.assertEquals( null, got, 'get, when called with a key that is not in the cache, will return null' ); + } + + @isTest + private static void get_whenCalledByUserWithCacheAccessWithAKeyThatWasPut_returnsIt() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + String expected = 'thecachedthing'; + new OrgCache().put( 'key', expected, DEFAULT_LIFESPAN ); + + Object got = new OrgCache().get( 'key' ); + System.assertEquals( expected, got, 'get, when called with a key that is in the cache, will return it' ); + } + + + @isTest + private static void put_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + OrgCache cache = new OrgCache(); + + Test.startTest(); + Exception exceptionThrown; + try + { + cache.put( 'a key', 'value', DEFAULT_LIFESPAN ); + } + catch ( OrgCache.AccessViolationException e ) + { + exceptionThrown = e; + } + Test.stopTest(); + + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'put, when the user does not have access to the cache, will throw an exception' ); + } + + @isTest + private static void put_whenCalledByUserWithCacheAccessMultipleTimesWithTheSameKey_overwritesIt() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().put( 'key', '1', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key', '2', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key', '3', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key', '4', DEFAULT_LIFESPAN ); + + Object got = new OrgCache().get( 'key' ); + System.assertEquals( '4', got, 'put, when called multiple times with the same key, overwrites it' ); + } + + @isTest + private static void put_whenCalledByUserWithCacheAccessMultipleTimesWithDifferentKeys_storesThem() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().put( 'key1', '1', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key2', '2', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key3', '3', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key4', '4', DEFAULT_LIFESPAN ); + + Object got = new OrgCache().get( 'key2' ); + System.assertEquals( '2', got, 'put, when called multiple times with different keys, stores them' ); + } + + + @isTest + private static void contains_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + OrgCache cache = new OrgCache(); + + Test.startTest(); + Exception exceptionThrown; + try + { + cache.contains( 'a key' ); + } + catch ( OrgCache.AccessViolationException e ) + { + exceptionThrown = e; + } + Test.stopTest(); + + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'contains, when the user does not have access to the cache, will throw an exception' ); + } + + @isTest + private static void contains_whenCalledByUserWithCacheAccessForAKeyThatWasPut_returnsTrue() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); + + Boolean got = new OrgCache().contains( 'keythatexists' ); + System.assertEquals( true, got, 'contains, when called for a key that was put, returns true' ); + } + + @isTest + private static void contains_whenCalledByUserWithCacheAccessForAKeyThatWasNotPut_returnsFalse() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); + + Boolean got = new OrgCache().contains( 'keythatdoesnotexist' ); + System.assertEquals( false, got, 'contains, when called for a key that was not put, returns false' ); + } + + @isTest + private static void remove_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + OrgCache cache = new OrgCache(); + + Test.startTest(); + Exception exceptionThrown; + try + { + cache.remove( 'a key' ); + } + catch ( OrgCache.AccessViolationException e ) + { + exceptionThrown = e; + } + Test.stopTest(); + + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'remove, when the user does not have access to the cache, will throw an exception' ); + } + + @isTest + private static void remove_whenCalledByUserWithCacheAccessForAKeyThatWasPut_removesIt() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().put( 'originalkey', '1', DEFAULT_LIFESPAN ); + + new OrgCache().remove( 'originalkey' ); + + System.assertEquals( false, new OrgCache().contains( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking contains' ); + System.assertEquals( null, new OrgCache().get( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking get' ); + } + + @isTest + private static void remove_whenCalledByUserWithCacheAccessForAKeyThatWasNotPut_doesNotError() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().remove( 'keythatneverexisted' ); + + System.assertEquals( false, new OrgCache().contains( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking contains' ); + System.assertEquals( null, new OrgCache().get( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking get' ); + } + + @isTest + private static void removeAll_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + OrgCache cache = new OrgCache(); + + Test.startTest(); + Exception exceptionThrown; + try + { + cache.removeAll(); + } + catch ( OrgCache.AccessViolationException e ) + { + exceptionThrown = e; + } + Test.stopTest(); + + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'remove, when the user does not have access to the cache, will throw an exception' ); + } + + @isTest + private static void removeAll_whenCalledByUserWithCacheAccess_willRemoveAllKeysFromTheCache() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().put( 'key1', '1', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key2', '2', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key3', '3', DEFAULT_LIFESPAN ); + + new OrgCache().removeAll(); + + System.assertEquals( false, new OrgCache().contains( 'key1' ), 'removeAll, when called for a key that was put, will remove it - checking contains - 1' ); + System.assertEquals( null, new OrgCache().get( 'key1' ), 'removeAll, when called for a key that was put, will remove it - checking get - 1' ); + + System.assertEquals( false, new OrgCache().contains( 'key2' ), 'removeAll, when called for a key that was put, will remove it - checking contains - 2' ); + System.assertEquals( null, new OrgCache().get( 'key2' ), 'removeAll, when called for a key that was put, will remove it - checking get - 3' ); + + System.assertEquals( false, new OrgCache().contains( 'key3' ), 'removeAll, when called for a key that was put, will remove it - checking contains - 3' ); + System.assertEquals( null, new OrgCache().get( 'key3' ), 'removeAll, when called for a key that was put, will remove it - checking get - 3' ); + } + + private static void setupAccessToSoqlCache( Boolean accessToCache ) + { + ApplicationMockRegistrar.registerMockService( IPermissionsService.class ) + .when( 'hasAccessToCorePlatformCache' ) + .returns( accessToCache ); + } +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + From 3b80f95359716311e1265571afd6d6359cf2eeda Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 15:44:09 +0000 Subject: [PATCH 15/28] Reformatted tests so they do not include fluent calls to the tested method This reduces false positives on checks for untested code --- .../caching/tests/OrgCacheTest.cls | 41 +++++++++++++------ .../tests/PermissionsServiceImplTest.cls | 4 +- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls index 113169b1207..18a920595b3 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls @@ -7,8 +7,9 @@ private without sharing class OrgCacheTest private static void hasAccessToCache_whenCalledByUserWithCacheAccess_returnsTrue() // NOPMD: Test method name format { setupAccessToSoqlCache( true ); + OrgCache cache = new OrgCache(); - Boolean got = new OrgCache().hasAccessToCache(); + Boolean got = cache.hasAccessToCache(); System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); } @@ -16,8 +17,9 @@ private without sharing class OrgCacheTest private static void hasAccessToCache_whenCalledByUserWithoutCacheAccess_returnsFalse() // NOPMD: Test method name format { setupAccessToSoqlCache( false ); + OrgCache cache = new OrgCache(); - Boolean got = new OrgCache().hasAccessToCache(); + Boolean got = cache.hasAccessToCache(); System.assertEquals( false, got, 'hasAccessToCache, when called, will return false' ); } @@ -26,7 +28,8 @@ private without sharing class OrgCacheTest { setupAccessToSoqlCache( true ); - Boolean got = new OrgCache().isACache(); + OrgCache cache = new OrgCache(); + Boolean got = cache.isACache(); System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); } @@ -35,7 +38,8 @@ private without sharing class OrgCacheTest { setupAccessToSoqlCache( false ); - Boolean got = new OrgCache().isACache(); + OrgCache cache = new OrgCache(); + Boolean got = cache.isACache(); System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); } @@ -66,7 +70,8 @@ private without sharing class OrgCacheTest { setupAccessToSoqlCache( true ); - Object got = new OrgCache().get( 'doesnotexist' ); + OrgCache cache = new OrgCache(); + Object got = cache.get( 'doesnotexist' ); System.assertEquals( null, got, 'get, when called with a key that is not in the cache, will return null' ); } @@ -78,7 +83,8 @@ private without sharing class OrgCacheTest String expected = 'thecachedthing'; new OrgCache().put( 'key', expected, DEFAULT_LIFESPAN ); - Object got = new OrgCache().get( 'key' ); + OrgCache cache = new OrgCache(); + Object got = cache.get( 'key' ); System.assertEquals( expected, got, 'get, when called with a key that is in the cache, will return it' ); } @@ -115,7 +121,8 @@ private without sharing class OrgCacheTest new OrgCache().put( 'key', '3', DEFAULT_LIFESPAN ); new OrgCache().put( 'key', '4', DEFAULT_LIFESPAN ); - Object got = new OrgCache().get( 'key' ); + OrgCache cache = new OrgCache(); + Object got = cache.get( 'key' ); System.assertEquals( '4', got, 'put, when called multiple times with the same key, overwrites it' ); } @@ -129,7 +136,8 @@ private without sharing class OrgCacheTest new OrgCache().put( 'key3', '3', DEFAULT_LIFESPAN ); new OrgCache().put( 'key4', '4', DEFAULT_LIFESPAN ); - Object got = new OrgCache().get( 'key2' ); + OrgCache cache = new OrgCache(); + Object got = cache.get( 'key2' ); System.assertEquals( '2', got, 'put, when called multiple times with different keys, stores them' ); } @@ -163,7 +171,8 @@ private without sharing class OrgCacheTest new OrgCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); - Boolean got = new OrgCache().contains( 'keythatexists' ); + OrgCache cache = new OrgCache(); + Boolean got = cache.contains( 'keythatexists' ); System.assertEquals( true, got, 'contains, when called for a key that was put, returns true' ); } @@ -174,7 +183,8 @@ private without sharing class OrgCacheTest new OrgCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); - Boolean got = new OrgCache().contains( 'keythatdoesnotexist' ); + OrgCache cache = new OrgCache(); + Boolean got = cache.contains( 'keythatdoesnotexist' ); System.assertEquals( false, got, 'contains, when called for a key that was not put, returns false' ); } @@ -207,7 +217,8 @@ private without sharing class OrgCacheTest new OrgCache().put( 'originalkey', '1', DEFAULT_LIFESPAN ); - new OrgCache().remove( 'originalkey' ); + OrgCache cache = new OrgCache(); + cache.remove( 'originalkey' ); System.assertEquals( false, new OrgCache().contains( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking contains' ); System.assertEquals( null, new OrgCache().get( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking get' ); @@ -218,7 +229,8 @@ private without sharing class OrgCacheTest { setupAccessToSoqlCache( true ); - new OrgCache().remove( 'keythatneverexisted' ); + OrgCache cache = new OrgCache(); + cache.remove( 'keythatneverexisted' ); System.assertEquals( false, new OrgCache().contains( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking contains' ); System.assertEquals( null, new OrgCache().get( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking get' ); @@ -255,7 +267,10 @@ private without sharing class OrgCacheTest new OrgCache().put( 'key2', '2', DEFAULT_LIFESPAN ); new OrgCache().put( 'key3', '3', DEFAULT_LIFESPAN ); - new OrgCache().removeAll(); + Test.startTest(); + OrgCache cache = new OrgCache(); + cache.removeAll(); + Test.stopTest(); System.assertEquals( false, new OrgCache().contains( 'key1' ), 'removeAll, when called for a key that was put, will remove it - checking contains - 1' ); System.assertEquals( null, new OrgCache().get( 'key1' ), 'removeAll, when called for a key that was put, will remove it - checking get - 1' ); diff --git a/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls b/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls index bd7f30ef4ed..5b6109936b7 100644 --- a/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls +++ b/framework/default/standard-services/default/classes/services/permissions-service/tests/PermissionsServiceImplTest.cls @@ -4,7 +4,9 @@ private with sharing class PermissionsServiceImplTest @isTest private static void hasAccessToCorePlatformCache_doesNotThrowAnException() // NOPMD: test method format { - Boolean hasPermission = new PermissionsServiceImpl().hasAccessToCorePlatformCache(); + PermissionsServiceImpl permissionsService = new PermissionsServiceImpl(); + + Boolean hasPermission = permissionsService.hasAccessToCorePlatformCache(); System.assertNotEquals( null, hasPermission, 'hasAccessToCorePlatformCache, does not throw an exception, and returns a value' ); } From e456765293a48c1353db5297a94ff8096cd82bc8 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 15:47:17 +0000 Subject: [PATCH 16/28] Reformatted tests to ensure Clayton picks up testing of the methods --- .../caching/tests/NullCacheTest.cls | 18 ++++++++---- .../caching/tests/SessionCacheTest.cls | 29 ++++++++++++------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls index a4c62694f59..f0a225f20a3 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls @@ -4,42 +4,48 @@ private without sharing class NullCacheTest @isTest private static void hasAccessToCache_whenCalled_returnsTrue() // NOPMD: Test method name format { - Boolean got = new NullCache().hasAccessToCache(); + NullCache cache = new NullCache(); + Boolean got = cache.hasAccessToCache(); System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); } @isTest private static void isACache_whenCalled_returnsFalse() // NOPMD: Test method name format { - Boolean got = new NullCache().isACache(); + NullCache cache = new NullCache(); + Boolean got = cache.isACache(); System.assertEquals( false, got, 'hasAccessToCache, when called, will return false' ); } @isTest private static void get_whenCalled_returnsNull() // NOPMD: Test method name format { - Object got = new NullCache().get( null ); + NullCache cache = new NullCache(); + Object got = cache.get( null ); System.assertEquals( null, got, 'get, when called, will return null' ); } @isTest private static void put_whenCalled_doesNothing() // NOPMD: Test method name format { - new NullCache().put( null, null, null ); + NullCache cache = new NullCache(); + cache.put( null, null, null ); System.assert( true, 'put, when called, will do nothing and not throw an exception' ); } @isTest private static void contains_whenCalled_returnsFalse() // NOPMD: Test method name format { - Boolean got = new NullCache().contains( null ); + NullCache cache = new NullCache(); + Boolean got = cache.contains( null ); System.assertEquals( false, got, 'contains, when called, will return false' ); } @isTest private static void remove_whenCalled_doesNothing() // NOPMD: Test method name format { - new NullCache().remove( null ); + NullCache cache = new NullCache(); + cache.remove( null ); System.assert( true, 'remove, when called, will do nothing and not throw an exception' ); } } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls index 5bcdfb4067e..92d2f517f15 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls @@ -6,21 +6,24 @@ private without sharing class SessionCacheTest @isTest private static void hasAccessToCache_whenCalled_returnsTrue() // NOPMD: Test method name format { - Boolean got = new SessionCache().hasAccessToCache(); + SessionCache cache = new SessionCache(); + Boolean got = cache.hasAccessToCache(); System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); } @isTest private static void isACache_whenCalled_returnsTrue() // NOPMD: Test method name format { - Boolean got = new SessionCache().isACache(); + SessionCache cache = new SessionCache(); + Boolean got = cache.isACache(); System.assertEquals( true, got, 'hasAccessToCache, when called, will return true' ); } @isTest private static void get_whenCalledWithAKeyNotInTheCache_returnsNull() // NOPMD: Test method name format { - Object got = new SessionCache().get( 'doesnotexist' ); + SessionCache cache = new SessionCache(); + Object got = cache.get( 'doesnotexist' ); System.assertEquals( null, got, 'get, when called with a key that is not in the cache, will return null' ); } @@ -30,7 +33,8 @@ private without sharing class SessionCacheTest String expected = 'thecachedthing'; new SessionCache().put( 'key', expected, DEFAULT_LIFESPAN ); - Object got = new SessionCache().get( 'key' ); + SessionCache cache = new SessionCache(); + Object got = cache.get( 'key' ); System.assertEquals( expected, got, 'get, when called with a key that is in the cache, will return it' ); } @@ -42,7 +46,8 @@ private without sharing class SessionCacheTest new SessionCache().put( 'key', '3', DEFAULT_LIFESPAN ); new SessionCache().put( 'key', '4', DEFAULT_LIFESPAN ); - Object got = new SessionCache().get( 'key' ); + SessionCache cache = new SessionCache(); + Object got = cache.get( 'key' ); System.assertEquals( '4', got, 'put, when called multiple times with the same key, overwrites it' ); } @@ -54,7 +59,8 @@ private without sharing class SessionCacheTest new SessionCache().put( 'key3', '3', DEFAULT_LIFESPAN ); new SessionCache().put( 'key4', '4', DEFAULT_LIFESPAN ); - Object got = new SessionCache().get( 'key2' ); + SessionCache cache = new SessionCache(); + Object got = cache.get( 'key2' ); System.assertEquals( '2', got, 'put, when called multiple times with different keys, stores them' ); } @@ -63,7 +69,8 @@ private without sharing class SessionCacheTest { new SessionCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); - Boolean got = new SessionCache().contains( 'keythatexists' ); + SessionCache cache = new SessionCache(); + Boolean got = cache.contains( 'keythatexists' ); System.assertEquals( true, got, 'contains, when called for a key that was put, returns true' ); } @@ -72,7 +79,8 @@ private without sharing class SessionCacheTest { new SessionCache().put( 'keythatexists', '1', DEFAULT_LIFESPAN ); - Boolean got = new SessionCache().contains( 'keythatdoesnotexist' ); + SessionCache cache = new SessionCache(); + Boolean got = cache.contains( 'keythatdoesnotexist' ); System.assertEquals( false, got, 'contains, when called for a key that was not put, returns false' ); } @@ -81,7 +89,8 @@ private without sharing class SessionCacheTest { new SessionCache().put( 'originalkey', '1', DEFAULT_LIFESPAN ); - new SessionCache().remove( 'originalkey' ); + SessionCache cache = new SessionCache(); + cache.remove( 'originalkey' ); System.assertEquals( false, new SessionCache().contains( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking contains' ); System.assertEquals( null, new SessionCache().get( 'originalkey' ), 'remove, when called for a key that was put, will remove it - checking get' ); @@ -90,7 +99,7 @@ private without sharing class SessionCacheTest @isTest private static void remove_whenCalledForAKeyThatWasNotPut_doesNotError() // NOPMD: Test method name format { - new SessionCache().remove( 'keythatneverexisted' ); + cache.remove( 'keythatneverexisted' ); System.assertEquals( false, new SessionCache().contains( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking contains' ); System.assertEquals( null, new SessionCache().get( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking get' ); From c743113a68eb6bbdc0abb966c212a58894afc0e9 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 15:53:20 +0000 Subject: [PATCH 17/28] Fixed compile error --- .../classes/fflib-extension/caching/tests/SessionCacheTest.cls | 1 + 1 file changed, 1 insertion(+) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls index 92d2f517f15..fb1b1c56b9c 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls @@ -99,6 +99,7 @@ private without sharing class SessionCacheTest @isTest private static void remove_whenCalledForAKeyThatWasNotPut_doesNotError() // NOPMD: Test method name format { + SessionCache cache = new SessionCache(); cache.remove( 'keythatneverexisted' ); System.assertEquals( false, new SessionCache().contains( 'keythatneverexisted' ), 'remove, when called for a key that was put, will remove it - checking contains' ); From 776d6917ec31d369fdb21470c2c22d5ff1f03d94 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Thu, 10 Mar 2022 16:32:33 +0000 Subject: [PATCH 18/28] Started to write and sketch the test for ObjectCache --- .../fflib-extension/caching/ObjectCache.cls | 1 + .../caching/tests/ObjectCacheTest.cls | 39 +++++++++++++++++++ .../tests/ObjectCacheTest.cls-meta.xml | 5 +++ 3 files changed, 45 insertions(+) create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls create mode 100644 framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls-meta.xml diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index c7c4e9dd83b..f700be3c9ec 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -76,6 +76,7 @@ public with sharing class ObjectCache String getIdFor( Object objectToGetIdFor ); } + @testVisible ICacheAdaptor cacheWrapper = new OrgCache(); // by default, configure the cache to use the org version private final static Integer CACHE_LIFESPAN_SECONDS = 28800; // TODO: soft setting / option diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls new file mode 100644 index 00000000000..70978f2c673 --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -0,0 +1,39 @@ +@isTest +private without sharing class ObjectCacheTest +{ + @isTest + private static void constructor_setsTheCacheToOrgCache() // NOPMD: Test method name format + { + Test.startTest(); + ObjectCache cache = new ObjectCache(); + Test.stopTest(); + + System.assert( cache.cacheWrapper instanceOf OrgCache, 'constructor, will set the cache to SessionCache' ); + } + + @isTest + private static void setScope_whenGivenOrg_setsTheCacheToOrgCache() // NOPMD: Test method name format + { + + ObjectCache cache = new ObjectCache(); + + Test.startTest(); + cache.setScope( ObjectCache.CacheScope.ORG ); + Test.stopTest(); + + System.assert( cache.cacheWrapper instanceOf OrgCache, 'setScope, when given Org, will set the cache to SessionCache' ); + } + + @isTest + private static void setScope_whenGivenSession_setsTheCacheToSessionCache() // NOPMD: Test method name format + { + + ObjectCache cache = new ObjectCache(); + + Test.startTest(); + cache.setScope( ObjectCache.CacheScope.SESSION ); + Test.stopTest(); + + System.assert( cache.cacheWrapper instanceOf SessionCache, 'setScope, when given SESSION, will set the cache to SessionCache' ); + } +} \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls-meta.xml b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls-meta.xml new file mode 100644 index 00000000000..dd61d1f917e --- /dev/null +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls-meta.xml @@ -0,0 +1,5 @@ + + + 52.0 + Active + From dc44f8d60b7534754d273062225ebd52b6453344 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Fri, 11 Mar 2022 15:32:15 +0000 Subject: [PATCH 19/28] Started testing get on objectCache --- .../fflib-extension/caching/ObjectCache.cls | 13 +- .../caching/tests/ObjectCacheTest.cls | 215 +++++++++++++++--- 2 files changed, 193 insertions(+), 35 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index f700be3c9ec..8f215414f20 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -10,12 +10,6 @@ */ public with sharing class ObjectCache { - // TODO: do we need to protect against 100kb limit - // TODO: could implement a longest time since accessed, first out - // TODO: could implement a random age out - // TODO: could just ignore and invalidate the cache - check with the rest of the seniors - // TODO: Docs on keys - public class InvalidIdentifierException extends Exceptions.DeveloperException {} public enum CacheScope { ORG, SESSION } @@ -118,6 +112,11 @@ public with sharing class ObjectCache Contract.requires( key != null, 'get called with a null key' ); Contract.requires( ids != null, 'get called with a null ids' ); + for ( String thisId : ids ) + { + Contract.requires( thisId != null, 'get called with a null entry in ids' ); + } + CacheRetrieval values = new CacheRetrieval(); Map cachedObjects = (Map)cacheWrapper.get( key ); @@ -129,8 +128,6 @@ public with sharing class ObjectCache for ( String thisId : ids ) { - Contract.assert( thisId != null, 'get called with a null entry in ids' ); - Object thisValue = cachedObjects.get( thisId ); if ( thisValue != null ) { diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index 70978f2c673..b890217bba9 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -1,39 +1,200 @@ @isTest private without sharing class ObjectCacheTest { - @isTest - private static void constructor_setsTheCacheToOrgCache() // NOPMD: Test method name format - { - Test.startTest(); - ObjectCache cache = new ObjectCache(); - Test.stopTest(); + @isTest + private static void constructor_setsTheCacheToOrgCache() // NOPMD: Test method name format + { + Test.startTest(); + ObjectCache cache = new ObjectCache(); + Test.stopTest(); - System.assert( cache.cacheWrapper instanceOf OrgCache, 'constructor, will set the cache to SessionCache' ); - } + System.assert( cache.cacheWrapper instanceOf OrgCache, 'constructor, will set the cache to SessionCache' ); + } - @isTest - private static void setScope_whenGivenOrg_setsTheCacheToOrgCache() // NOPMD: Test method name format - { + @isTest + private static void setScope_whenGivenOrg_setsTheCacheToOrgCache() // NOPMD: Test method name format + { - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache(); - Test.startTest(); - cache.setScope( ObjectCache.CacheScope.ORG ); - Test.stopTest(); + Test.startTest(); + cache.setScope( ObjectCache.CacheScope.ORG ); + Test.stopTest(); - System.assert( cache.cacheWrapper instanceOf OrgCache, 'setScope, when given Org, will set the cache to SessionCache' ); - } + System.assert( cache.cacheWrapper instanceOf OrgCache, 'setScope, when given Org, will set the cache to SessionCache' ); + } - @isTest - private static void setScope_whenGivenSession_setsTheCacheToSessionCache() // NOPMD: Test method name format - { + @isTest + private static void setScope_whenGivenSession_setsTheCacheToSessionCache() // NOPMD: Test method name format + { - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache(); - Test.startTest(); - cache.setScope( ObjectCache.CacheScope.SESSION ); - Test.stopTest(); + Test.startTest(); + cache.setScope( ObjectCache.CacheScope.SESSION ); + Test.stopTest(); + + System.assert( cache.cacheWrapper instanceOf SessionCache, 'setScope, when given SESSION, will set the cache to SessionCache' ); + } + + @isTest + private static void get_whenGivenAStringKeyThatDoesNotExist_willReturnAllAsCacheMisses() // NOPMD: Test method name format + { + Set ids = new Set{ 'missId1', 'missId2' }; + + ObjectCache cache = new ObjectCache(); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'nonekey', ids ); + Test.stopTest(); + + System.assertEquals( ids, retrieval.cacheMisses, 'get, when given a string key that does not exist, will return all as cache misses' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'get, when given a string key that does not exist, will return no cache hits' ); + } + + @isTest + private static void get_whenGivenStringIdsThatDoNotExist_willReturnAllAsCacheMisses() // NOPMD: Test method name format + { + Set ids = new Set{ 'miss1Id', 'miss2Id' }; + + ObjectCache cache = new ObjectCache(); + cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', ids ); + Test.stopTest(); + + System.assertEquals( ids, retrieval.cacheMisses, 'get, when given string ids that do not exist in the given key, will return all as cache misses' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'get, when given string ids that do not exist in the given key, will return no cache hits' ); + } + + @isTest + private static void get_whenGivenSomeStringIdsThatExist_willReturnHitsAndMisses() // NOPMD: Test method name format + { + Set ids = new Set{ 'miss1Id', 'miss2Id', 'hit1Id', 'hit2Id' }; + + ObjectCache cache = new ObjectCache(); + cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', ids ); + Test.stopTest(); + + System.assertEquals( new Set{ 'miss1Id', 'miss2Id' }, retrieval.cacheMisses, 'get, when given some string ids that exist and some that do not, will return those that do not as cache misses' ); + System.assertEquals( new Set{ 'hit1Id', 'hit2Id' }, retrieval.cacheHits.keySet(), 'get, when given some string ids that exist and some that do not, will return those that do indexed by their id (checking id)' ); + System.assertEquals( new List{ 'hit1', 'hit2' }, retrieval.cacheHits.values(), 'get, when given some string ids that exist and some that do not, will return those that do indexed by their id (checking content)' ); + } + + @isTest + private static void get_whenGivenAllStringIdsThatExist_willReturnHitsAndNoMisses() // NOPMD: Test method name format + { + Set ids = new Set{ 'hit1Id', 'hit2Id' }; + + ObjectCache cache = new ObjectCache(); + cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', ids ); + Test.stopTest(); + + System.assertEquals( new Set(), retrieval.cacheMisses, 'get, when given all string ids that exist, will return no cache misses' ); + System.assertEquals( new Set{ 'hit1Id', 'hit2Id' }, retrieval.cacheHits.keySet(), 'get, when given all string ids that exist, will return those that do indexed by their id (checking id)' ); + System.assertEquals( new List{ 'hit1', 'hit2' }, retrieval.cacheHits.values(), 'get, when given all string ids that exist, will return those that do indexed by their id (checking content)' ); + } + + @isTest + private static void get_whenGivenANullKey_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache(); + + Test.startTest(); + String exceptionMessage; + try + { + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', new Set{ 'id1', null, 'id2' } ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'get called with a null entry in ids', exceptionMessage, 'get, when given a null key, will throw an exception' ); + } + + @isTest + private static void get_whenGivenSomeSobjectIdsThatExist_willReturnHitsAndMisses() // NOPMD: Test method name format + { + Map contacts = new Map{ + 'hit1' => new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ), + 'hit2' => new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ), + 'miss1' => new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ), + 'miss2' => new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ) + }; + + Set ids = new Set{ + contacts.get( 'hit1' ).Id, + contacts.get( 'hit2' ).Id, + contacts.get( 'miss1' ).Id, + contacts.get( 'miss2' ).Id + }; + + ObjectCache cache = new ObjectCache(); + cache.put( 'key', new List{ contacts.get( 'hit1' ), contacts.get( 'hit2' ) } ); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', ids ); + Test.stopTest(); + + Set expectedMisses = new Set{ contacts.get( 'miss1' ).Id, contacts.get( 'miss2' ).Id }; + Set expectedHitIds = new Set{ contacts.get( 'hit1' ).Id, contacts.get( 'hit2' ).Id }; + List expectedHits = new List{ contacts.get( 'hit1' ), contacts.get( 'hit2' ) }; + + System.assertEquals( expectedMisses, retrieval.cacheMisses, 'get, when given some ids that exist and some that do not, will return those that do not as cache misses' ); + System.assertEquals( expectedHitIds, retrieval.cacheHits.keySet(), 'get, when given some ids that exist and some that do not, will return those that do indexed by their id (checking id)' ); + System.assertEquals( expectedHits[0], retrieval.cacheHits.values()[0], 'get, when given some ids that exist and some that do not, will return those that do indexed by their id (checking content)' ); + System.assertEquals( expectedHits[1], retrieval.cacheHits.values()[1], 'get, when given some ids that exist and some that do not, will return those that do indexed by their id (checking content)' ); + } + + // get, when given a single string key that exists, will return it as a cache hit + // get, when given a single string key that does not exist, will return it as a cache miss + // get, when given a single id key that exists, will return it as a cache hit + // get, when given a single id key that does not exist, will return it as a cache miss + + // put, when given a list of sobjects, will cache them by their id + // put, when given a list of sobjects, and one is null, will throw an exception + // put, when given a list of sobjects, and one has a null id, will throw an exception + // put, when given a list of sobjects and a name of a string field, will cache the sobjects by the value in that field + // put, when given a list of sobjects and a name of a string field, and one has a null value, will throw an exception + + // put, when given a list of sobjects and the name of a non string field, will throw an exception + + // put, when given a list of objects and an idgetter, will cache them based on the return of the idgetter + // put, when given a list of objects and an idgetter, and one of the objects is null, will throw an exception + // put, when given a list of objects and an idgetter, and one of the objects returns a null id, will throw an exception + + // put, when given a list of objects and an idgetter, when objects are already in the cache, will not overwrite ones not referenced twice + // put, when given a list of objects and an idgetter, when objects are already in the cache, will not overwrite ones that are referenced twice + + // put, when called multiple times with different base keys will not overwrite the other keys + + // remove, when called with a key that exists, will remove all objects from the cache for that key + // remove, when called with a key that does not exist, will not throw an exception + + // remove, when called with a key and and a set of Ids that exist, will remove the ones that do, leave the others + // remove, when called with a key and and a set of Ids that do not exist, will not throw an exception + + // remove, when called with a key and and a set of Strings that exist, will remove the ones that do, leave the others + // remove, when called with a key and and a set of Strings that do not exist, will not throw an exception + + public class StringIdGetter implements ObjectCache.IdGetter + { + public String getIdFor( Object objectToGetIdFor ) + { + return (String)objectToGetIdFor + 'Id'; + } + } + + +} - System.assert( cache.cacheWrapper instanceOf SessionCache, 'setScope, when given SESSION, will set the cache to SessionCache' ); - } -} \ No newline at end of file From 475bf3351fe1b47b9cc6aebb2d6e198a3736d8d1 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Fri, 11 Mar 2022 15:38:53 +0000 Subject: [PATCH 20/28] Fixed unit test fragility --- .../caching/tests/ObjectCacheTest.cls | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index b890217bba9..3370462daec 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -27,7 +27,6 @@ private without sharing class ObjectCacheTest @isTest private static void setScope_whenGivenSession_setsTheCacheToSessionCache() // NOPMD: Test method name format { - ObjectCache cache = new ObjectCache(); Test.startTest(); @@ -42,7 +41,7 @@ private without sharing class ObjectCacheTest { Set ids = new Set{ 'missId1', 'missId2' }; - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); Test.startTest(); ObjectCache.CacheRetrieval retrieval = cache.get( 'nonekey', ids ); @@ -57,7 +56,7 @@ private without sharing class ObjectCacheTest { Set ids = new Set{ 'miss1Id', 'miss2Id' }; - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); Test.startTest(); @@ -73,7 +72,7 @@ private without sharing class ObjectCacheTest { Set ids = new Set{ 'miss1Id', 'miss2Id', 'hit1Id', 'hit2Id' }; - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); Test.startTest(); @@ -90,7 +89,7 @@ private without sharing class ObjectCacheTest { Set ids = new Set{ 'hit1Id', 'hit2Id' }; - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); Test.startTest(); @@ -105,7 +104,7 @@ private without sharing class ObjectCacheTest @isTest private static void get_whenGivenANullKey_throwsAnException() // NOPMD: Test method name format { - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); Test.startTest(); String exceptionMessage; @@ -139,7 +138,7 @@ private without sharing class ObjectCacheTest contacts.get( 'miss2' ).Id }; - ObjectCache cache = new ObjectCache(); + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); cache.put( 'key', new List{ contacts.get( 'hit1' ), contacts.get( 'hit2' ) } ); Test.startTest(); From 36dcd3df3ffd8ba1699e039acb86552c5a87bbf7 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Fri, 11 Mar 2022 16:20:33 +0000 Subject: [PATCH 21/28] Adding more testing, removing some defunct code and clarifying error cases --- .../fflib-extension/caching/ObjectCache.cls | 8 +- .../caching/tests/ObjectCacheTest.cls | 307 ++++++++++++++++-- 2 files changed, 289 insertions(+), 26 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index 8f215414f20..6f0af2d7f45 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -211,6 +211,7 @@ public with sharing class ObjectCache return put( key, new SobjectIdGetter( idField ), sobjects ); } + // TODO: implement put( String key, IdGetter idGetter, Object objectToStore ) /** * Put the given Objects into the cache, using the given base key combined with the value that is returned from * each object when the IdGetter is called against it @@ -304,11 +305,6 @@ public with sharing class ObjectCache this.idField = idField; } - private SobjectIdGetter() - { - this( 'Id' ); - } - public String getIdFor( Object objectToGetIdFor ) { try @@ -317,7 +313,7 @@ public with sharing class ObjectCache } catch ( Exception e ) { - throw new InvalidIdentifierException( 'Unable to retrieve the String Identifier from the field ' + idField + ' from the SObject: ' + objectToGetIdFor, e ) + throw new InvalidIdentifierException( 'Unable to retrieve the String Identifier from the field ' + idField + ' from the SObject: ' + objectToGetIdFor + ' - maybe it is not a String or Id?', e ) .setErrorCode( FrameworkErrorCodes.UNABLE_TO_RETRIEVE_IDENTIFIER ) .addContext( 'object', objectToGetIdFor ) .addContext( 'idField', idField ); diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index 3370462daec..bfdccc76abc 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -79,8 +79,8 @@ private without sharing class ObjectCacheTest ObjectCache.CacheRetrieval retrieval = cache.get( 'key', ids ); Test.stopTest(); - System.assertEquals( new Set{ 'miss1Id', 'miss2Id' }, retrieval.cacheMisses, 'get, when given some string ids that exist and some that do not, will return those that do not as cache misses' ); - System.assertEquals( new Set{ 'hit1Id', 'hit2Id' }, retrieval.cacheHits.keySet(), 'get, when given some string ids that exist and some that do not, will return those that do indexed by their id (checking id)' ); + System.assertEquals( new Set{ 'miss1Id', 'miss2Id' }, retrieval.cacheMisses, 'get, when given some string ids that exist and some that do not, will return those that do not as cache misses' ); + System.assertEquals( new Set{ 'hit1Id', 'hit2Id' }, retrieval.cacheHits.keySet(), 'get, when given some string ids that exist and some that do not, will return those that do indexed by their id (checking id)' ); System.assertEquals( new List{ 'hit1', 'hit2' }, retrieval.cacheHits.values(), 'get, when given some string ids that exist and some that do not, will return those that do indexed by their id (checking content)' ); } @@ -97,7 +97,7 @@ private without sharing class ObjectCacheTest Test.stopTest(); System.assertEquals( new Set(), retrieval.cacheMisses, 'get, when given all string ids that exist, will return no cache misses' ); - System.assertEquals( new Set{ 'hit1Id', 'hit2Id' }, retrieval.cacheHits.keySet(), 'get, when given all string ids that exist, will return those that do indexed by their id (checking id)' ); + System.assertEquals( new Set{ 'hit1Id', 'hit2Id' }, retrieval.cacheHits.keySet(), 'get, when given all string ids that exist, will return those that do indexed by their id (checking id)' ); System.assertEquals( new List{ 'hit1', 'hit2' }, retrieval.cacheHits.values(), 'get, when given all string ids that exist, will return those that do indexed by their id (checking content)' ); } @@ -155,27 +155,273 @@ private without sharing class ObjectCacheTest System.assertEquals( expectedHits[1], retrieval.cacheHits.values()[1], 'get, when given some ids that exist and some that do not, will return those that do indexed by their id (checking content)' ); } - // get, when given a single string key that exists, will return it as a cache hit - // get, when given a single string key that does not exist, will return it as a cache miss - // get, when given a single id key that exists, will return it as a cache hit - // get, when given a single id key that does not exist, will return it as a cache miss + @isTest + private static void get_whenGivenASingleStringIdThatExists_willReturnAsAHit() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', 'hit1Id' ); + Test.stopTest(); + + System.assertEquals( new Set(), retrieval.cacheMisses, 'get, when given a string id that exists, will return no cache misses' ); + System.assertEquals( new Set{ 'hit1Id' }, retrieval.cacheHits.keySet(), 'get, when given a string id that exists, will return it as a hit indexed by their id (checking id)' ); + System.assertEquals( new List{ 'hit1' }, retrieval.cacheHits.values(), 'get, when given a string id that exists, will return it as a hit indexed by their id (checking content)' ); + } + + @isTest + private static void get_whenGivenASingleStringIdThatDoesNotExist_willReturnAsAMiss() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + cache.put( 'key', new StringIdGetter(), new List{ 'hit1', 'hit2' } ); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', 'miss1' ); + Test.stopTest(); + + System.assertEquals( new Set{ 'miss1' }, retrieval.cacheMisses, 'get, when given a string id that does not exist, will return it as a cache miss' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'get, when given a single string id does not exist, will return no hits' ); + } + + @isTest + private static void get_whenGivenASingleIdThatExists_willReturnAsAHit() // NOPMD: Test method name format + { + Map contacts = new Map{ + 'hit1' => new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ), + 'hit2' => new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ) + }; + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + cache.put( 'key', contacts.values() ); - // put, when given a list of sobjects, will cache them by their id - // put, when given a list of sobjects, and one is null, will throw an exception - // put, when given a list of sobjects, and one has a null id, will throw an exception - // put, when given a list of sobjects and a name of a string field, will cache the sobjects by the value in that field - // put, when given a list of sobjects and a name of a string field, and one has a null value, will throw an exception + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', contacts.get( 'hit1' ).Id ); + Test.stopTest(); - // put, when given a list of sobjects and the name of a non string field, will throw an exception + System.assertEquals( new Set(), retrieval.cacheMisses, 'get, when given an id that exists, will return no cache misses' ); + System.assertEquals( new Set{ contacts.get( 'hit1' ).Id }, retrieval.cacheHits.keySet(), 'get, when given an id that exists, will return it as a hit indexed by their id (checking id)' ); + System.assertEquals( contacts.get( 'hit1' ), retrieval.cacheHits.values()[0], 'get, when given an id that exists, will return it as a hit indexed by their id (checking content)' ); + } - // put, when given a list of objects and an idgetter, will cache them based on the return of the idgetter - // put, when given a list of objects and an idgetter, and one of the objects is null, will throw an exception - // put, when given a list of objects and an idgetter, and one of the objects returns a null id, will throw an exception + @isTest + private static void get_whenGivenASingleIdThatDoesNotExist_willReturnAsAMiss() // NOPMD: Test method name format + { + List hitContacts = new List{ + new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ) + }; - // put, when given a list of objects and an idgetter, when objects are already in the cache, will not overwrite ones not referenced twice - // put, when given a list of objects and an idgetter, when objects are already in the cache, will not overwrite ones that are referenced twice + Contact missContact = new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ); - // put, when called multiple times with different base keys will not overwrite the other keys + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + cache.put( 'key', hitContacts ); + + Test.startTest(); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', missContact.Id ); + Test.stopTest(); + + System.assertEquals( new Set{ missContact.Id }, retrieval.cacheMisses, 'get, when given an id that does not exist, will return it as a cache miss' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'get, when given an id does not exist, will return no hits' ); + } + + @isTest + private static void put_whenGivenSObjectsAndOneIsNull_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', new List{ + new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ), + null, + new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ) + }); + } + catch ( Exception e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'put called with a null entry in objects', exceptionMessage, 'put, when given SObjects and one is null, will throw an exception' ); + } + + @isTest + private static void put_whenGivenSObjectsAndOneHasNullId_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', new List{ + new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ), + new Contact(), + new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ) + }); + } + catch ( Exception e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'put called with an object that has a null Id', exceptionMessage, 'put, when given SObjects and one has a null Id, will throw an exception' ); + } + + @isTest + private static void put_whenGivenAListOfSobjectsAndAValidStringFieldName_willCacheBasedOnThePassedField() // NOPMD: Test method name format + { + List contacts = new List{ + new Contact( LastName = 'hit1' ), + new Contact( LastName = 'hit2' ) + }; + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + cache.put( 'key', 'LastName', contacts ); + Test.stopTest(); + + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', 'hit1' ); + + System.assertEquals( contacts[0], retrieval.cacheHits.get( 'hit1' ), 'put, when given a list of sobjects and a valid string field name, will index by the field passed' ); + } + + @isTest + private static void put_whenGivenSobjectsAndAnInvalidFieldName_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', 'INVALID_FIELD_NAME', new List{ new Contact( LastName = 'hit1' ) }); + } + catch ( ObjectCache.InvalidIdentifierException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'Unable to retrieve the String Identifier from the field INVALID_FIELD_NAME from the SObject: Contact', exceptionMessage, 'put, when given a list of sobjects and an invalid field name, will throw an exception' ); + } + + @isTest + private static void put_whenGivenSobjectsAndANonStringFieldName_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', 'MailingLatitude', new List{ new Contact( MailingLatitude = 0 ) }); + } + catch ( ObjectCache.InvalidIdentifierException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'Unable to retrieve the String Identifier from the field MailingLatitude from the SObject: Contact', exceptionMessage, 'put, when given a list of sobjects and an invalid field name, will throw an exception' ); + } + + @isTest + private static void put_whenGivenSobjectsAndASobjectWithNullField_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', 'LastName', new List{ + new Contact( LastName = 'hit1' ), + new Contact() + }); + } + catch ( Contract.AssertException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'put called with an object that has a null Id: Contact', exceptionMessage, 'put, when given a list of sobjects and an invalid field name, will throw an exception' ); + } + + @isTest + private static void put_whenGivenObjectAndOneWithANullId_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', new StringIdGetter(), new List{ + 'hit1', + 'nullid', + 'hit2' + }); + } + catch ( Contract.AssertException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'put called with an object that has a null Id: nullid', exceptionMessage, 'put, when given a list of sobjects and an invalid field name, will throw an exception' ); + } + + @isTest + private static void put_whenGivenDifferentKeysWithTheSameIds_willNotOverwriteEachOther() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + String OBJECT_ID = 'id1'; + + List objectsToStore = new List{ + new ObjectToStore( OBJECT_ID, 'value1' ), + new ObjectToStore( OBJECT_ID, 'value2' ) + }; + + Test.startTest(); + cache.put( 'key1', new ObjectIdGetter(), new List{ objectsToStore[0] } ); + cache.put( 'key2', new ObjectIdGetter(), new List{ objectsToStore[1] } ); + Test.stopTest(); + + ObjectCache.CacheRetrieval retrieval; + + retrieval = cache.get( 'key1', OBJECT_ID ); + System.assertEquals( objectsToStore[0], retrieval.cacheHits.get( OBJECT_ID ), 'put, when given objects with the same ids, but different keys, will index them independently' ); + + retrieval = cache.get( 'key2', OBJECT_ID ); + System.assertEquals( objectsToStore[1], retrieval.cacheHits.get( OBJECT_ID ), 'put, when given objects with the same ids, but different keys, will index them independently (checking index 2)' ); + } + + @isTest + private static void put_whenGivenDifferentObjectsWithTheSameKeyAndId_willOverwriteEachOther() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + String OBJECT_ID = 'id1'; + + List objectsToStore = new List{ + new ObjectToStore( OBJECT_ID, 'value1' ), + new ObjectToStore( OBJECT_ID, 'value2' ) + }; + + Test.startTest(); + cache.put( 'key1', new ObjectIdGetter(), new List{ objectsToStore[0] } ); + cache.put( 'key1', new ObjectIdGetter(), new List{ objectsToStore[1] } ); + Test.stopTest(); + + ObjectCache.CacheRetrieval retrieval; + + retrieval = cache.get( 'key1', OBJECT_ID ); + System.assertEquals( objectsToStore[1], retrieval.cacheHits.get( OBJECT_ID ), 'put, when given objects with the same ids and keys, the latter will overwrite the earlier' ); + } // remove, when called with a key that exists, will remove all objects from the cache for that key // remove, when called with a key that does not exist, will not throw an exception @@ -190,10 +436,31 @@ private without sharing class ObjectCacheTest { public String getIdFor( Object objectToGetIdFor ) { + if ( objectToGetIdFor == 'nullid' ) + { + return null; + } return (String)objectToGetIdFor + 'Id'; } } + public class ObjectToStore + { + public String id; + public String value; -} + public ObjectToStore( String id, String value ) + { + this.id = id; + this.value = value; + } + } + public class ObjectIdGetter implements ObjectCache.IdGetter + { + public String getIdFor( Object objectToGetIdFor ) + { + return ((ObjectToStore)objectToGetIdFor).id; + } + } +} \ No newline at end of file From 00823a147ba0ca4877b7e0d47af78bcc55dd01d4 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Mon, 14 Mar 2022 09:02:26 +0000 Subject: [PATCH 22/28] Started to test 'remove' methods --- .../caching/tests/ObjectCacheTest.cls | 55 ++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index bfdccc76abc..46a38df7cfe 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -423,8 +423,59 @@ private without sharing class ObjectCacheTest System.assertEquals( objectsToStore[1], retrieval.cacheHits.get( OBJECT_ID ), 'put, when given objects with the same ids and keys, the latter will overwrite the earlier' ); } - // remove, when called with a key that exists, will remove all objects from the cache for that key - // remove, when called with a key that does not exist, will not throw an exception + @isTest + private static void remove_whenGivenAKeyThatExists_willRemoveAllObjectsFromThatKey() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', new StringIdGetter(), new List{ 'value1', 'value2', 'value3' } ); + + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).remove( 'key' ); + Test.stopTest(); + + ObjectCache.CacheRetrieval retrieval; + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value1Id' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'remove, when given a key that exists, will remove all objects from that key - 1' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value2Id' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'remove, when given a key that exists, will remove all objects from that key - 2' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value3Id' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'remove, when given a key that exists, will remove all objects from that key - 3' ); + } + + @isTest + private static void remove_whenGivenAKeyThatExists_willNotRemoveObjectsFromOtherKeys() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', new StringIdGetter(), new List{ 'value1', 'value2', 'value3' } ); + + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).remove( 'otherKey' ); + Test.stopTest(); + + ObjectCache.CacheRetrieval retrieval; + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value1Id' ); + System.assertEquals( 1, retrieval.cacheHits.size(), 'remove, when given a key that exists, will not remove obejcts from other keys - 1' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value2Id' ); + System.assertEquals( 1, retrieval.cacheHits.size(), 'remove, when given a key that exists, will not remove obejcts from other keys - 2' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value3Id' ); + System.assertEquals( 1, retrieval.cacheHits.size(), 'remove, when given a key that exists, will not remove obejcts from other keys - 3' ); + } + + @isTest + private static void remove_whenGivenAKeyThatDoesNotExist_willNotThrowAnExecption() // NOPMD: Test method name format + { + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).remove( 'key' ); + Test.stopTest(); + + System.assert( true, 'remove, when given a key that does not exist, will not throw an exception' ); + } // remove, when called with a key and and a set of Ids that exist, will remove the ones that do, leave the others // remove, when called with a key and and a set of Ids that do not exist, will not throw an exception From 7bc1289ae5440ed77b30f0589938a959370fcdd7 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Mon, 14 Mar 2022 09:10:14 +0000 Subject: [PATCH 23/28] Finished ObjectCacheTest --- .../fflib-extension/caching/ObjectCache.cls | 6 ++ .../caching/tests/ObjectCacheTest.cls | 73 +++++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index 6f0af2d7f45..d09e2dda259 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -286,6 +286,12 @@ public with sharing class ObjectCache Contract.requires( ids != null, 'remove called with a null ids' ); Map cachedObjects = (Map)cacheWrapper.get( key ); + + if ( cachedObjects == null ) + { + return this; + } + for ( String thisId : ids ) { Contract.assert( thisId != null, 'remove called with a null entry in ids' ); diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index 46a38df7cfe..1d0d53bd03d 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -14,7 +14,6 @@ private without sharing class ObjectCacheTest @isTest private static void setScope_whenGivenOrg_setsTheCacheToOrgCache() // NOPMD: Test method name format { - ObjectCache cache = new ObjectCache(); Test.startTest(); @@ -477,11 +476,75 @@ private without sharing class ObjectCacheTest System.assert( true, 'remove, when given a key that does not exist, will not throw an exception' ); } - // remove, when called with a key and and a set of Ids that exist, will remove the ones that do, leave the others - // remove, when called with a key and and a set of Ids that do not exist, will not throw an exception + @isTest + private static void remove_whenGivenAKeyAndStringIdsThatExist_willRemoveTheOnesThatExist() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', new StringIdGetter(), new List{ 'value1', 'value2', 'value3' } ); + + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).remove( 'key', new Set{ 'value1Id', 'value3Id' } ); + Test.stopTest(); + + ObjectCache.CacheRetrieval retrieval; + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value1Id' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'remove, when given a key and String ids that exist, will remove the ones that exist - 1' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value2Id' ); + System.assertEquals( 1, retrieval.cacheHits.size(), 'remove, when given a key and String ids that exist, will not remove the ones that are not specified' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', 'value3Id' ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'remove, when given a key and String ids that exist, will remove the ones that exist - 2' ); + } + + @isTest + private static void remove_whenGivenAKeyAndStringIdCombinationThatDoesNotExist_willNotThrowAnExecption() // NOPMD: Test method name format + { + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).remove( 'key', new Set{ 'value1Id', 'value3Id' } ); + Test.stopTest(); + + System.assert( true, 'remove, when given a key and String Id combination that does not exist, will not throw an exception' ); + } + + @isTest + private static void remove_whenGivenAKeyAndIdsThatExist_willRemoveTheOnesThatExist() // NOPMD: Test method name format + { + List contacts = new List{ + new Contact( Id = TestIdUtils.generateId( Contact.sobjectType ) ), + new Contact( Id = TestIdUtils.generateId( Contact.sobjectType ) ), + new Contact( Id = TestIdUtils.generateId( Contact.sobjectType ) ) + }; + + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', contacts ); + + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).remove( 'key', new Set{ contacts[0].Id, contacts[2].Id } ); + Test.stopTest(); - // remove, when called with a key and and a set of Strings that exist, will remove the ones that do, leave the others - // remove, when called with a key and and a set of Strings that do not exist, will not throw an exception + ObjectCache.CacheRetrieval retrieval; + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', contacts[0].Id ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'remove, when given a key and ids that exist, will remove the ones that exist - 1' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', contacts[1].Id ); + System.assertEquals( 1, retrieval.cacheHits.size(), 'remove, when given a key and ids that exist, will not remove the ones that are not specified' ); + + retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).get( 'key', contacts[2].Id ); + System.assertEquals( 0, retrieval.cacheHits.size(), 'remove, when given a key and ids that exist, will remove the ones that exist - 2' ); + } + + @isTest + private static void remove_whenGivenAKeyAndIdCombinationThatDoesNotExist_willNotThrowAnExecption() // NOPMD: Test method name format + { + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ).remove( 'key', new Set{ TestIdUtils.generateId( Contact.SobjectType ) } ); + Test.stopTest(); + + System.assert( true, 'remove, when given a key and Id combination that does not exist, will not throw an exception' ); + } public class StringIdGetter implements ObjectCache.IdGetter { From 72e1eb98431eb7edaf875b84730c3aa63a331f51 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Mon, 14 Mar 2022 09:42:38 +0000 Subject: [PATCH 24/28] Simplify ObjectCache to use a cache key per put --- .../fflib-extension/caching/ICacheAdaptor.cls | 1 + .../fflib-extension/caching/NullCache.cls | 5 ++ .../fflib-extension/caching/ObjectCache.cls | 67 +++++++++++-------- .../fflib-extension/caching/OrgCache.cls | 22 +++++- .../fflib-extension/caching/SessionCache.cls | 12 +++- .../caching/tests/ObjectCacheTest.cls | 3 + 6 files changed, 78 insertions(+), 32 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls index 75d4a510d9b..a145dcd70c1 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ICacheAdaptor.cls @@ -15,4 +15,5 @@ public interface ICacheAdaptor void put( String key, Object value, Integer lifespan ); Boolean contains( String key ); void remove( String key ); + Set getKeys(); } \ No newline at end of file diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls index c2bf10eb631..7e3d189cc44 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/NullCache.cls @@ -22,6 +22,11 @@ public inherited sharing class NullCache implements ICacheAdaptor return null; } + public Set getKeys() + { + return new Set(); + } + public void put( String key, Object value, Integer lifespan ) // NOPMD: Intentionally left empty, as this should do nothing in a NullCache { } diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index d09e2dda259..9097067d8d3 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -8,7 +8,7 @@ * Note: The lifespan of a given object is reset whenever any object in that object's list is written. * That is, either the whole of the list is aged out, or none of it is. */ -public with sharing class ObjectCache +public inherited sharing class ObjectCache { public class InvalidIdentifierException extends Exceptions.DeveloperException {} @@ -119,16 +119,9 @@ public with sharing class ObjectCache CacheRetrieval values = new CacheRetrieval(); - Map cachedObjects = (Map)cacheWrapper.get( key ); - - if ( cachedObjects == null ) - { - return values.setCacheMisses( ids ); - } - for ( String thisId : ids ) { - Object thisValue = cachedObjects.get( thisId ); + Object thisValue = cacheWrapper.get( buildFullKey( key, thisId ) ); if ( thisValue != null ) { values.addCacheHit( thisId, thisValue ); @@ -212,6 +205,9 @@ public with sharing class ObjectCache } // TODO: implement put( String key, IdGetter idGetter, Object objectToStore ) + // TODO: implement put( String key, String idField, Sobject sobjectToStore ) + // TODO: implement put( String key, Sobject sobjectToStore ) + /** * Put the given Objects into the cache, using the given base key combined with the value that is returned from * each object when the IdGetter is called against it @@ -227,22 +223,15 @@ public with sharing class ObjectCache Contract.requires( idGetter != null, 'put called with a null idGetter' ); Contract.requires( objects != null, 'put called with a null objects' ); - Map cachedObjects = (Map)cacheWrapper.get( key ); - - if ( cachedObjects == null ) - { - cachedObjects = new Map(); - } - for ( Object thisObject : objects ) { Contract.assert( thisObject != null, 'put called with a null entry in objects' ); String thisId = idGetter.getIdFor( thisObject ); Contract.assert( thisId != null, 'put called with an object that has a null Id: ' + thisObject ); - cachedObjects.put( thisId, thisObject ); + + cacheWrapper.put( buildFullKey( key, thisId ), thisObject, CACHE_LIFESPAN_SECONDS ); } - cacheWrapper.put( key, cachedObjects, CACHE_LIFESPAN_SECONDS ); return this; } @@ -253,10 +242,19 @@ public with sharing class ObjectCache * @param String The base key to remove the objects from. * @return ObjectCache Itself, allowing for a fluent interface */ - public ObjectCache remove( String key ) + public ObjectCache remove( String keyToRemove ) { - Contract.requires( key != null, 'remove called with a null key' ); - cacheWrapper.remove( key ); + Contract.requires( keyToRemove != null, 'remove called with a null keyToRemove' ); + + Set allExistingKeys = cacheWrapper.getKeys(); + + for ( String thisExistingKey : allExistingKeys ) + { + if ( isForKey( thisExistingKey, keyToRemove ) ) + { + cacheWrapper.remove( thisExistingKey ); + } + } return this; } @@ -285,23 +283,34 @@ public with sharing class ObjectCache Contract.requires( key != null, 'remove called with a null key' ); Contract.requires( ids != null, 'remove called with a null ids' ); - Map cachedObjects = (Map)cacheWrapper.get( key ); - - if ( cachedObjects == null ) + // This is two loops because we want to check all the ids before we remove anything + for ( String thisId : ids ) { - return this; + Contract.assert( thisId != null, 'remove called with a null entry in ids' ); } for ( String thisId : ids ) { - Contract.assert( thisId != null, 'remove called with a null entry in ids' ); - - cachedObjects.remove( thisId ); + cacheWrapper.remove( buildFullKey( key, thisId ) ); } - cacheWrapper.put( key, cachedObjects, CACHE_LIFESPAN_SECONDS ); return this; } + private String buildFullKey( String key, String id ) + { + return buildKeyPrefix( key ) + id; + } + + private Boolean isForKey( String fullKey, String key ) + { + return fullKey.startsWith( buildKeyPrefix( key ) ); + } + + private String buildKeyPrefix( String key ) + { + return key + 'xXXx'; + } + private class SobjectIdGetter implements IdGetter { String idField; diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index a8445d987a7..852edb80194 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -70,6 +70,24 @@ public inherited sharing class OrgCache implements ICacheAdaptor return Cache.Org.get( createFullyQualifiedKey( key ) ); } + /** + * Retrieve all the keys that are currently stored in the cache. + * + * If the user does not have access to the cache, will throw an AccessViolationException. + * + * @return Set The keys for all cached objects + */ + public Set getKeys() + { + if ( ! hasAccessToCache ) + { + throw new AccessViolationException( Label.ortoo_core_org_cache_access_violation ) + .setErrorCode( FrameworkErrorCodes.CACHE_ACCESS_VIOLATION ) + .addContext( 'method', 'getKeys' ); + } + return Cache.Org.getKeys(); + } + /** * Put the stated value into the stated key for the specified duration (in seconds) * @@ -122,7 +140,7 @@ public inherited sharing class OrgCache implements ICacheAdaptor } /** - * Instucts the cache to remove the object at the given key + * Instructs the cache to remove the object at the given key * * If the user does not have access to the cache, will throw an AccessViolationException. * @@ -143,7 +161,7 @@ public inherited sharing class OrgCache implements ICacheAdaptor } /** - * Instucts the cache to remove all objects from the cache. + * Instructs the cache to remove all objects from the cache. * * Is useful for clearing the cache out completely when the majority of entries would otherwise be dirty. * diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls index 90c8441d770..9b3b53a9768 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls @@ -39,6 +39,16 @@ public inherited sharing class SessionCache implements ICacheAdaptor return Cache.Session.get( key ); } + /** + * Retrieve all the keys that are currently stored in the cache. + * + * @return Set The keys for all cached objects + */ + public Set getKeys() + { + return Cache.Session.getKeys(); + } + /** * Put the stated value into the stated key for the specified duration (in seconds) * @@ -72,7 +82,7 @@ public inherited sharing class SessionCache implements ICacheAdaptor } /** - * Instucts the cache to remove the object at the given key + * Instructs the cache to remove the object at the given key * * @param String The key to remove */ diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index 1d0d53bd03d..14874a7c592 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -546,6 +546,9 @@ private without sharing class ObjectCacheTest System.assert( true, 'remove, when given a key and Id combination that does not exist, will not throw an exception' ); } +// TODO: remove called with a null / empty id will throw an exception +// TODO: remove called with a null / empty id will not remove other things + public class StringIdGetter implements ObjectCache.IdGetter { public String getIdFor( Object objectToGetIdFor ) From 6d1013eec7f9f4ef562ac5db598a36f23a3671dc Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Mon, 14 Mar 2022 10:17:15 +0000 Subject: [PATCH 25/28] Added tests for getKeys --- .../caching/tests/NullCacheTest.cls | 8 ++++ .../caching/tests/OrgCacheTest.cls | 37 +++++++++++++++++++ .../caching/tests/SessionCacheTest.cls | 15 ++++++++ 3 files changed, 60 insertions(+) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls index f0a225f20a3..e1edc93de90 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/NullCacheTest.cls @@ -33,6 +33,14 @@ private without sharing class NullCacheTest System.assert( true, 'put, when called, will do nothing and not throw an exception' ); } + @isTest + private static void getKeys_whenCalled_returnsAnEmptySet() // NOPMD: Test method name format + { + NullCache cache = new NullCache(); + Set got = cache.getKeys(); + System.assertEquals( 0, got.size(), 'getKeys, when called, will return an empty set' ); + } + @isTest private static void contains_whenCalled_returnsFalse() // NOPMD: Test method name format { diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls index 18a920595b3..9c2559f7f3a 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/OrgCacheTest.cls @@ -141,6 +141,43 @@ private without sharing class OrgCacheTest System.assertEquals( '2', got, 'put, when called multiple times with different keys, stores them' ); } + @isTest + private static void getKeyss_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format + { + setupAccessToSoqlCache( false ); + + OrgCache cache = new OrgCache(); + + Test.startTest(); + Exception exceptionThrown; + try + { + cache.getKeys(); + } + catch ( OrgCache.AccessViolationException e ) + { + exceptionThrown = e; + } + Test.stopTest(); + + ortoo_Asserts.assertContains( Label.ortoo_core_org_cache_access_violation, exceptionThrown?.getMessage(), 'getKeys, when the user does not have access to the cache, will throw an exception' ); + } + + @isTest + private static void getKeys_whenCalledByUserWithCacheAccess_returnsTheKeysInTheCache() // NOPMD: Test method name format + { + setupAccessToSoqlCache( true ); + + new OrgCache().put( 'key1', '1', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key2', '2', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key3', '3', DEFAULT_LIFESPAN ); + new OrgCache().put( 'key4', '4', DEFAULT_LIFESPAN ); + + OrgCache cache = new OrgCache(); + Object got = cache.getKeys(); + + System.assertEquals( new Set{ 'key1', 'key2', 'key3', 'key4' }, got, 'getKeys, when called by user with cache access, returns the keys in the cache' ); + } @isTest private static void contains_whenTheUserDoesNotHaveAccessToTheCache_throwsAnException() // NOPMD: Test method name format diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls index fb1b1c56b9c..8b89febd68e 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/SessionCacheTest.cls @@ -64,6 +64,21 @@ private without sharing class SessionCacheTest System.assertEquals( '2', got, 'put, when called multiple times with different keys, stores them' ); } + + @isTest + private static void getKeys_returnsTheKeysInTheCache() // NOPMD: Test method name format + { + new SessionCache().put( 'key1', '1', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key2', '2', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key3', '3', DEFAULT_LIFESPAN ); + new SessionCache().put( 'key4', '4', DEFAULT_LIFESPAN ); + + SessionCache cache = new SessionCache(); + Object got = cache.getKeys(); + + System.assertEquals( new Set{ 'key1', 'key2', 'key3', 'key4' }, got, 'getKeys, returns the keys in the cache' ); + } + @isTest private static void contains_whenCalledForAKeyThatWasPut_returnsTrue() // NOPMD: Test method name format { From c40e5807a8d4af5bf173765589aa27796bd01865 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Mon, 14 Mar 2022 10:47:08 +0000 Subject: [PATCH 26/28] Remove chance of blank keys and ids in the cache Added blank and null key behaviour tests for remove --- .../fflib-extension/caching/ObjectCache.cls | 31 ++++--- .../fflib-extension/caching/OrgCache.cls | 8 +- .../fflib-extension/caching/SessionCache.cls | 8 +- .../caching/tests/ObjectCacheTest.cls | 83 ++++++++++++++++--- 4 files changed, 98 insertions(+), 32 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index 9097067d8d3..87f622778b1 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -109,12 +109,12 @@ public inherited sharing class ObjectCache */ public CacheRetrieval get( String key, Set ids ) { - Contract.requires( key != null, 'get called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'get called with a blank key' ); Contract.requires( ids != null, 'get called with a null ids' ); for ( String thisId : ids ) { - Contract.requires( thisId != null, 'get called with a null entry in ids' ); + Contract.requires( String.isNotBlank( thisId ), 'get called with a blank entry in ids' ); } CacheRetrieval values = new CacheRetrieval(); @@ -157,7 +157,7 @@ public inherited sharing class ObjectCache */ public CacheRetrieval get( String key, String id ) { - Contract.requires( id != null, 'get called with a null id' ); + Contract.requires( String.isNotBlank( id ), 'get called with a blank id' ); return get( key, new Set{ id } ); } @@ -171,8 +171,8 @@ public inherited sharing class ObjectCache */ public CacheRetrieval get( String key, Id id ) { - Contract.requires( key != null, 'get called with a null key' ); - Contract.requires( id != null, 'get called with a null id' ); + Contract.requires( String.isNotBlank( key ), 'get called with a blank key' ); + Contract.requires( String.isNotBlank( id ), 'get called with a blank id' ); return get( key, new Set{ id } ); } @@ -200,7 +200,7 @@ public inherited sharing class ObjectCache */ public ObjectCache put( String key, String idField, List sobjects ) { - Contract.requires( idField != null, 'put called with a null idField' ); + Contract.requires( String.isNotBlank( idField ), 'put called with a blank idField' ); return put( key, new SobjectIdGetter( idField ), sobjects ); } @@ -219,17 +219,22 @@ public inherited sharing class ObjectCache */ public ObjectCache put( String key, IdGetter idGetter, List objects ) { - Contract.requires( key != null, 'put called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'put called with a blank key' ); Contract.requires( idGetter != null, 'put called with a null idGetter' ); Contract.requires( objects != null, 'put called with a null objects' ); for ( Object thisObject : objects ) { - Contract.assert( thisObject != null, 'put called with a null entry in objects' ); - String thisId = idGetter.getIdFor( thisObject ); + Contract.requires( thisObject != null, 'put called with a null entry in objects' ); - Contract.assert( thisId != null, 'put called with an object that has a null Id: ' + thisObject ); + String thisId = idGetter.getIdFor( thisObject ); + Contract.requires( String.isNotBlank( thisId ), 'put called with an object that has a blank Id: ' + thisObject ); + } + // Done in two loops so we check that everything is valid before writing anything + for ( Object thisObject : objects ) + { + String thisId = idGetter.getIdFor( thisObject ); cacheWrapper.put( buildFullKey( key, thisId ), thisObject, CACHE_LIFESPAN_SECONDS ); } @@ -244,7 +249,7 @@ public inherited sharing class ObjectCache */ public ObjectCache remove( String keyToRemove ) { - Contract.requires( keyToRemove != null, 'remove called with a null keyToRemove' ); + Contract.requires( String.isNotBlank( keyToRemove ), 'remove called with a blank keyToRemove' ); Set allExistingKeys = cacheWrapper.getKeys(); @@ -280,13 +285,13 @@ public inherited sharing class ObjectCache */ public ObjectCache remove( String key, Set ids ) { - Contract.requires( key != null, 'remove called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'remove called with a blank key' ); Contract.requires( ids != null, 'remove called with a null ids' ); // This is two loops because we want to check all the ids before we remove anything for ( String thisId : ids ) { - Contract.assert( thisId != null, 'remove called with a null entry in ids' ); + Contract.requires( String.isNotBlank( thisId ), 'remove called with a blank entry in ids' ); } for ( String thisId : ids ) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls index 852edb80194..8e34d31ba22 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/OrgCache.cls @@ -57,7 +57,7 @@ public inherited sharing class OrgCache implements ICacheAdaptor */ public Object get( String key ) { - Contract.requires( key != null, 'get called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'get called with a blank key' ); if ( ! hasAccessToCache ) { @@ -100,7 +100,7 @@ public inherited sharing class OrgCache implements ICacheAdaptor */ public void put( String key, Object value, Integer lifespan ) { - Contract.requires( key != null, 'put called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'put called with a blank key' ); Contract.requires( lifespan != null, 'put called with a null lifespan' ); Contract.requires( value != null, 'put called with a null value' ); Contract.requires( lifespan >= 0, 'put called with a negaitve lifespan' ); @@ -127,7 +127,7 @@ public inherited sharing class OrgCache implements ICacheAdaptor */ public Boolean contains( String key ) { - Contract.requires( key != null, 'contains called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'contains called with a blank key' ); if ( ! hasAccessToCache ) { @@ -148,7 +148,7 @@ public inherited sharing class OrgCache implements ICacheAdaptor */ public void remove( String key ) { - Contract.requires( key != null, 'remove called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'remove called with a blank key' ); if ( ! hasAccessToCache ) { diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls index 9b3b53a9768..1abb8b6873a 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/SessionCache.cls @@ -35,7 +35,7 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public Object get( String key ) { - Contract.requires( key != null, 'get called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'get called with a blank key' ); return Cache.Session.get( key ); } @@ -59,7 +59,7 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public void put( String key, Object value, Integer lifespan ) { - Contract.requires( key != null, 'put called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'put called with a blank key' ); Contract.requires( value != null, 'put called with a null value (call remove instead)' ); Contract.requires( lifespan != null, 'put called with a null lifespan' ); Contract.requires( lifespan >= 0, 'put called with a negative lifespan' ); @@ -76,7 +76,7 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public Boolean contains( String key ) { - Contract.requires( key != null, 'contains called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'contains called with a blank key' ); return Cache.Session.contains( key ); } @@ -88,7 +88,7 @@ public inherited sharing class SessionCache implements ICacheAdaptor */ public void remove( String key ) { - Contract.requires( key != null, 'remove called with a null key' ); + Contract.requires( String.isNotBlank( key ), 'remove called with a blank key' ); Cache.Session.remove( key ); } diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index 14874a7c592..baee24dfb94 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -101,7 +101,7 @@ private without sharing class ObjectCacheTest } @isTest - private static void get_whenGivenANullKey_throwsAnException() // NOPMD: Test method name format + private static void get_whenGivenABlankKey_throwsAnException() // NOPMD: Test method name format { ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); @@ -109,7 +109,7 @@ private without sharing class ObjectCacheTest String exceptionMessage; try { - ObjectCache.CacheRetrieval retrieval = cache.get( 'key', new Set{ 'id1', null, 'id2' } ); + ObjectCache.CacheRetrieval retrieval = cache.get( 'key', new Set{ 'id1', ' ', 'id2' } ); } catch ( Contract.RequiresException e ) { @@ -117,7 +117,7 @@ private without sharing class ObjectCacheTest } Test.stopTest(); - ortoo_Asserts.assertContains( 'get called with a null entry in ids', exceptionMessage, 'get, when given a null key, will throw an exception' ); + ortoo_Asserts.assertContains( 'get called with a blank entry in ids', exceptionMessage, 'get, when given a blank key, will throw an exception' ); } @isTest @@ -267,7 +267,7 @@ private without sharing class ObjectCacheTest } Test.stopTest(); - ortoo_Asserts.assertContains( 'put called with an object that has a null Id', exceptionMessage, 'put, when given SObjects and one has a null Id, will throw an exception' ); + ortoo_Asserts.assertContains( 'put called with an object that has a blank Id', exceptionMessage, 'put, when given SObjects and one has a blank Id, will throw an exception' ); } @isTest @@ -325,7 +325,7 @@ private without sharing class ObjectCacheTest } Test.stopTest(); - ortoo_Asserts.assertContains( 'Unable to retrieve the String Identifier from the field MailingLatitude from the SObject: Contact', exceptionMessage, 'put, when given a list of sobjects and an invalid field name, will throw an exception' ); + ortoo_Asserts.assertContains( 'Unable to retrieve the String Identifier from the field MailingLatitude from the SObject: Contact', exceptionMessage, 'put, when given a list of sobjects and a non string field name, will throw an exception' ); } @isTest @@ -342,13 +342,13 @@ private without sharing class ObjectCacheTest new Contact() }); } - catch ( Contract.AssertException e ) + catch ( Contract.RequiresException e ) { exceptionMessage = e.getMessage(); } Test.stopTest(); - ortoo_Asserts.assertContains( 'put called with an object that has a null Id: Contact', exceptionMessage, 'put, when given a list of sobjects and an invalid field name, will throw an exception' ); + ortoo_Asserts.assertContains( 'put called with an object that has a blank Id: Contact', exceptionMessage, 'put, when given a list of sobjects and one with a null Id will throw an exception' ); } @isTest @@ -366,13 +366,13 @@ private without sharing class ObjectCacheTest 'hit2' }); } - catch ( Contract.AssertException e ) + catch ( Contract.RequiresException e ) { exceptionMessage = e.getMessage(); } Test.stopTest(); - ortoo_Asserts.assertContains( 'put called with an object that has a null Id: nullid', exceptionMessage, 'put, when given a list of sobjects and an invalid field name, will throw an exception' ); + ortoo_Asserts.assertContains( 'put called with an object that has a blank Id: nullid', exceptionMessage, 'put, when given a list of sobjects one with a null Id, will throw an exception' ); } @isTest @@ -546,8 +546,69 @@ private without sharing class ObjectCacheTest System.assert( true, 'remove, when given a key and Id combination that does not exist, will not throw an exception' ); } -// TODO: remove called with a null / empty id will throw an exception -// TODO: remove called with a null / empty id will not remove other things + @isTest + private static void remove_whenGivenANullKey_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.remove( 'key', new Set{ 'id1', ' ', 'id2' } ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'remove called with a blank entry in ids', exceptionMessage, 'remove, when given a blank key, will throw an exception' ); + } + + @isTest + private static void remove_whenGivenABlankKey_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.remove( 'key', new Set{ 'id1', ' ', 'id2' } ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'remove called with a blank entry in ids', exceptionMessage, 'remove, when given a blank key, will throw an exception' ); + } + + @isTest + private static void remove_whenGivenABlankKey_willNotRemoveTheOtherKeys() + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', new StringIdGetter(), new List{ 'value1', 'value2', 'value3' } ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.remove( 'key', new Set{ 'value1Id', ' ', 'value3Id' } ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ObjectCache.CacheRetrieval retrieval = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .get( 'key', new Set{ 'value1Id', 'value2Id', 'value3Id' } ); + + System.assertEquals( 3, retrieval.cacheHits.size(), 'remove, when given a mix of blank and non-blank keys, will not remove the blank ones' ); + } public class StringIdGetter implements ObjectCache.IdGetter { From 678acfd654416c3785750535c466c9ad2db6e416 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Mon, 14 Mar 2022 11:29:21 +0000 Subject: [PATCH 27/28] Added single object puts --- .../fflib-extension/caching/ObjectCache.cls | 65 +++++++++++++++++-- .../caching/tests/ObjectCacheTest.cls | 64 ++++++++++++++++++ 2 files changed, 124 insertions(+), 5 deletions(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index 87f622778b1..cb9d41fab8e 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -182,13 +182,25 @@ public inherited sharing class ObjectCache * * @param String The base key to put the objects into * @param List The SObjects to store - * @return ObjectCache Itself, allowing for a fluent interface + * @return ObjectCache Itself, allowing for a fluent interface */ public ObjectCache put( String key, List sobjects ) { return put( key, 'Id', sobjects ); } + /** + * Put the given SObject into the cache, using the given base key combined with the SObject's Id + * + * @param String The base key to put the objects into + * @param Sobject The SObject to store + * @return ObjectCache Itself, allowing for a fluent interface + */ + public ObjectCache put( String key, Sobject sobjectToStore ) + { + return put( key, new List{ sobjectToStore } ); + } + /** * Put the given SObjects into the cache, using the given base key combined with the value stored * in the SObject field as defined by idField @@ -204,9 +216,19 @@ public inherited sharing class ObjectCache return put( key, new SobjectIdGetter( idField ), sobjects ); } - // TODO: implement put( String key, IdGetter idGetter, Object objectToStore ) - // TODO: implement put( String key, String idField, Sobject sobjectToStore ) - // TODO: implement put( String key, Sobject sobjectToStore ) + /** + * Put the given SObject into the cache, using the given base key combined with the value stored + * in the SObject field as defined by idField + * + * @param String The base key to put the object into + * @param String The field to get the SObject identifiersfrom (field value must be a non-null String) + * @param Sobject The SObject to store + * @return ObjectCache Itself, allowing for a fluent interface + */ + public ObjectCache put( String key, String idField, Sobject sobjectToStore ) + { + return put( key, idField, new List{ sobjectToStore } ); + } /** * Put the given Objects into the cache, using the given base key combined with the value that is returned from @@ -235,12 +257,45 @@ public inherited sharing class ObjectCache for ( Object thisObject : objects ) { String thisId = idGetter.getIdFor( thisObject ); - cacheWrapper.put( buildFullKey( key, thisId ), thisObject, CACHE_LIFESPAN_SECONDS ); + put( key, thisId, thisObject ); } return this; } + /** + * Put the given Object into the cache, using the given base key combined with the value that is returned from + * the object when the IdGetter is called against it + * + * @param String The base key to put the object into + * @param IdGetter The mechanism for getting the Id from the object + * @param Object The Object to store + * @return ObjectCache Itself, allowing for a fluent interface + */ + public ObjectCache put( String key, IdGetter idGetter, Object objectToStore ) + { + return put( key, idGetter, new List{ objectToStore } ); + } + + /** + * Put the given Object into the cache, using the given base key combined with given id + * + * @param String The base key to put the object into + * @param String The Id within the key to put the object into + * @param Object The Object to store + * @return ObjectCache Itself, allowing for a fluent interface + */ + public ObjectCache put( String key, String id, Object objectToStore ) + { + Contract.requires( String.isNotBlank( key ), 'put called with a blank key' ); + Contract.requires( String.isNotBlank( id ), 'put called with a blank id' ); + Contract.requires( objectToStore != null, 'put called with a null objectToStore' ); + + cacheWrapper.put( buildFullKey( key, id ), objectToStore, CACHE_LIFESPAN_SECONDS ); + + return this; + } + /** * Remove all the cached objects from the given base key. * diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index baee24dfb94..65c9f81b0f0 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -422,6 +422,70 @@ private without sharing class ObjectCacheTest System.assertEquals( objectsToStore[1], retrieval.cacheHits.get( OBJECT_ID ), 'put, when given objects with the same ids and keys, the latter will overwrite the earlier' ); } + @isTest + private static void put_whenGivenASingleSobject_storesIt() // NOPMD: Test method name format + { + Contact contact = new Contact( Id = TestIdUtils.generateId( Contact.SobjectType ) ); + + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', contact ); + + Test.stopTest(); + + ObjectCache.cacheRetrieval got = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .get( 'key', contact.Id ); + + System.assertEquals( contact, (Contact)got.cacheHits.get( contact.Id ), 'put, when given a single SObject, will store it' ); + } + + @isTest + private static void put_whenGivenASingleSobjectAndAFieldName_storesIt() // NOPMD: Test method name format + { + Contact contact = new Contact( AccountId = TestIdUtils.generateId( Account.SobjectType ) ); + + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', 'AccountId', contact ); + + Test.stopTest(); + + ObjectCache.cacheRetrieval got = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .get( 'key', contact.AccountId ); + + System.assertEquals( contact, (Contact)got.cacheHits.get( contact.AccountId ), 'put, when given a single SObject and a field name, will store it' ); + } + + @isTest + private static void put_whenGivenASingleObjectAndAnIdGetter_storesIt() // NOPMD: Test method name format + { + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', new StringIdGetter(), 'value1' ); + + Test.stopTest(); + + ObjectCache.cacheRetrieval got = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .get( 'key', 'value1Id' ); + + System.assertEquals( 'value1', got.cacheHits.get( 'value1Id' ), 'put, when given a single object and an id getter, will store it' ); + } + + @isTest + private static void put_whenGivenASingleObjectAndAnId_storesIt() // NOPMD: Test method name format + { + Test.startTest(); + new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .put( 'key', 'id1', 'value1' ); + + Test.stopTest(); + + ObjectCache.cacheRetrieval got = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ) + .get( 'key', 'id1' ); + + System.assertEquals( 'value1', got.cacheHits.get( 'id1' ), 'put, when given a single object and an id, will store it' ); + } + @isTest private static void remove_whenGivenAKeyThatExists_willRemoveAllObjectsFromThatKey() // NOPMD: Test method name format { From aedd0a14e7a91ea53b792e7d21eabb6184d959f9 Mon Sep 17 00:00:00 2001 From: Robert Baillie Date: Mon, 14 Mar 2022 11:56:45 +0000 Subject: [PATCH 28/28] Added exception tests for put --- .../fflib-extension/caching/ObjectCache.cls | 2 +- .../caching/tests/ObjectCacheTest.cls | 62 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls index cb9d41fab8e..9be2b90d047 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/ObjectCache.cls @@ -274,7 +274,7 @@ public inherited sharing class ObjectCache */ public ObjectCache put( String key, IdGetter idGetter, Object objectToStore ) { - return put( key, idGetter, new List{ objectToStore } ); + return put( key, idGetter.getIdFor( objectToStore ), objectToStore ); } /** diff --git a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls index 65c9f81b0f0..166f935c85c 100644 --- a/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls +++ b/framework/default/ortoo-core/default/classes/fflib-extension/caching/tests/ObjectCacheTest.cls @@ -486,6 +486,68 @@ private without sharing class ObjectCacheTest System.assertEquals( 'value1', got.cacheHits.get( 'id1' ), 'put, when given a single object and an id, will store it' ); } + @isTest + private static void put_whenGivenASingleObjectAndAnEmptyKey_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( ' ', 'id', 'value' ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'put called with a blank key', exceptionMessage, 'put, when given a single object and an empty key, will throw an exception' ); + } + + @isTest + private static void put_whenGivenASingleObjectAndAnEmptyId_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', ' ', 'value' ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'put called with a blank id', exceptionMessage, 'put, when given a single object and an empty id, will throw an exception' ); + } + + @isTest + private static void put_whenGivenASingleNullObject_throwsAnException() // NOPMD: Test method name format + { + ObjectCache cache = new ObjectCache().setScope( ObjectCache.CacheScope.SESSION ); + + Object nullObject = null; + + Test.startTest(); + String exceptionMessage; + try + { + cache.put( 'key', 'id', nullObject ); + } + catch ( Contract.RequiresException e ) + { + exceptionMessage = e.getMessage(); + } + Test.stopTest(); + + ortoo_Asserts.assertContains( 'put called with a null objectToStore', exceptionMessage, 'put, when given a single object that is null, will throw an exception' ); + } + @isTest private static void remove_whenGivenAKeyThatExists_willRemoveAllObjectsFromThatKey() // NOPMD: Test method name format {