Skip to content

Commit

Permalink
Merge pull request #2660 from mgillian/feat/RDBMcache
Browse files Browse the repository at this point in the history
feat/RDBMcache add 3 caches for high usage queries
  • Loading branch information
loulou2u committed May 17, 2023
2 parents 5c12180 + 13476cd commit e8db822
Show file tree
Hide file tree
Showing 6 changed files with 279 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.apereo.portal.portlet.om.IPortletEntity;
import org.apereo.portal.portlet.om.IPortletEntityId;
import org.apereo.portal.portlet.om.IPortletPreference;
import org.springframework.cache.annotation.CachePut;
import org.springframework.cache.annotation.Cacheable;

/**
* Wrapper for portlet entities that are persistent. Overrides the entity ID to be a consistent
Expand Down Expand Up @@ -69,7 +71,12 @@ public int getUserId() {
}

@Override
@Cacheable(
key = "windowStates",
cacheNames =
"org.apereo.portal.portlet.registry.PersistentPortletEntityWrapper.windowStates")
public Map<Long, WindowState> getWindowStates() {
// https://github.com/uPortal-Project/uPortal/issues/1903 #1
return this.persistentEntity.getWindowStates();
}

Expand All @@ -79,6 +86,10 @@ public WindowState getWindowState(IStylesheetDescriptor stylesheetDescriptor) {
}

@Override
@CachePut(
key = "windowStates",
cacheNames =
"org.apereo.portal.portlet.registry.PersistentPortletEntityWrapper.windowStates")
public void setWindowState(IStylesheetDescriptor stylesheetDescriptor, WindowState state) {
this.persistentEntity.setWindowState(stylesheetDescriptor, state);
}
Expand Down
2 changes: 2 additions & 0 deletions uPortal-groups/uPortal-groups-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ dependencies {
implementation project(':uPortal-rdbm')

implementation "com.google.guava:guava:${guavaVersion}"
// https://mvnrepository.com/artifact/org.mockito/mockito-core
testImplementation 'org.mockito:mockito-core:3.12.4'

compileOnly "${servletApiDependency}"
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ public class RDBMEntityGroupStore implements IEntityGroupStore, IGroupConstants
private static final String MEMBER_IS_GROUP = "T";

private static final String CACHE_NAME = "org.apereo.portal.groups.RDBMEntityGroupStore.search";
private static final String PARENT_GROUP_BY_ENTITY_CACHE_NAME =
"org.apereo.portal.groups.RDBMEntityGroupStore.parentGroupEntity";
private static final String PARENT_GROUP_BY_ENTTITY_GROUP_CACHE_NAME =
"org.apereo.portal.groups.RDBMEntityGroupStore.parentGroupEntityGroup";

// SQL group search string
private static final String SEARCH_GROUPS_PARTIAL_CASE_INSENSITIVE =
Expand Down Expand Up @@ -151,6 +155,8 @@ public class RDBMEntityGroupStore implements IEntityGroupStore, IGroupConstants

// Group search cache
private Ehcache groupSearchCache;
private Ehcache parentGroupEntityCache;
private Ehcache parentGroupEntityGroupCache;

/** RDBMEntityGroupStore constructor. */
public RDBMEntityGroupStore() {
Expand All @@ -174,18 +180,20 @@ private void initialize() {
}

// Cache for group search
groupSearchCache = getGroupSearchCache();
groupSearchCache = getGroupSearchCache(CACHE_NAME);
parentGroupEntityCache = getGroupSearchCache(PARENT_GROUP_BY_ENTITY_CACHE_NAME);
parentGroupEntityGroupCache = getGroupSearchCache(PARENT_GROUP_BY_ENTTITY_GROUP_CACHE_NAME);
}

private Ehcache getGroupSearchCache() {
private Ehcache getGroupSearchCache(String cacheName) {
final ApplicationContext context = ApplicationContextLocator.getApplicationContext();
assert context != null;
final CacheManager cacheManager = context.getBean("cacheManager", CacheManager.class);
assert cacheManager != null;
if (!cacheManager.cacheExists(CACHE_NAME)) {
cacheManager.addCache(CACHE_NAME);
if (!cacheManager.cacheExists(cacheName)) {
cacheManager.addCache(cacheName);
}
return cacheManager.getCache(CACHE_NAME);
return cacheManager.getCache(cacheName);
}

/**
Expand Down Expand Up @@ -333,9 +341,19 @@ public IEntityGroup find(String groupID) throws GroupsException {
* @return java.util.Iterator
*/
public java.util.Iterator findParentGroups(IEntity ent) throws GroupsException {
// https://github.com/uPortal-Project/uPortal/issues/1903 #2
String memberKey = ent.getKey();
Element el = parentGroupEntityCache.get(memberKey);
if (el != null) {
java.util.Iterator it = (java.util.Iterator) el.getObjectValue();
assert it != null;
return it;
}

Integer type = EntityTypesLocator.getEntityTypes().getEntityIDFromType(ent.getLeafType());
return findParentGroupsForEntity(memberKey, type.intValue());
java.util.Iterator it = findParentGroupsForEntity(memberKey, type.intValue());
parentGroupEntityCache.put(new Element(memberKey, it));
return it;
}

/**
Expand All @@ -345,10 +363,20 @@ public java.util.Iterator findParentGroups(IEntity ent) throws GroupsException {
* @return java.util.Iterator
*/
public java.util.Iterator findParentGroups(IEntityGroup group) throws GroupsException {
// https://github.com/uPortal-Project/uPortal/issues/1903 #8
String memberKey = group.getLocalKey();
Element el = parentGroupEntityGroupCache.get(memberKey);
if (el != null) {
java.util.Iterator it = (java.util.Iterator) el.getObjectValue();
assert it != null;
return it;
}

String serviceName = group.getServiceName().toString();
Integer type = EntityTypesLocator.getEntityTypes().getEntityIDFromType(group.getLeafType());
return findParentGroupsForGroup(serviceName, memberKey, type.intValue());
java.util.Iterator it = findParentGroupsForGroup(serviceName, memberKey, type.intValue());
parentGroupEntityGroupCache.put(new Element(memberKey, it));
return it;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
package org.apereo.portal.groups;

import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.util.ArrayList;
import java.util.Collection;
import javax.naming.Name;
import net.sf.ehcache.Cache;
import net.sf.ehcache.CacheManager;
import net.sf.ehcache.Element;
import org.apereo.portal.EntityTypes;
import org.apereo.portal.jdbc.RDBMServices;
import org.apereo.portal.spring.locator.ApplicationContextLocator;
import org.apereo.portal.spring.locator.EntityTypesLocator;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.MockedStatic;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.context.ApplicationContext;

@RunWith(MockitoJUnitRunner.class)
public class RDBMSEntityGroupStoreTest {
private static final String CACHE_NAME = "org.apereo.portal.groups.RDBMEntityGroupStore.search";
private static final String PARENT_GROUP_BY_ENTITY_CACHE_NAME =
"org.apereo.portal.groups.RDBMEntityGroupStore.parentGroupEntity";
private static final String PARENT_GROUP_BY_ENTTITY_GROUP_CACHE_NAME =
"org.apereo.portal.groups.RDBMEntityGroupStore.parentGroupEntityGroup";

@Test
public void testFindParentGroupsByIEntity() throws Exception {
// This test confirms that calling findParentGroups(IEntity) with the same key
// multiple times will check the cache each time but will only write
// to the cache one time.
//
// This test reflects the complexity of the RDBMEntityGroupStore class
// The RDBMEntityGroupStore class should really be two separate classes:
// a repository for reading from the database
// a service for processing the results.
//
// In addition, RDBMEntityGroupStore should be migrated to Spring annotations
// and bean management, which would eliminate the need for CacheManager
// and ApplicationContext
String cacheKey = "cache-key";
try (MockedStatic<ApplicationContextLocator> applicationContextLocator =
mockStatic(ApplicationContextLocator.class);
MockedStatic<RDBMServices> rdbmServices = mockStatic(RDBMServices.class);
MockedStatic<EntityTypesLocator> entityTypesLocator =
mockStatic(EntityTypesLocator.class)) {
// define all mocks
ApplicationContext context = mock(ApplicationContext.class);
Cache groupSearchCache = mock(Cache.class);
Cache parentGroupEntityCache = mock(Cache.class);
Cache parentGroupEntityGroupCache = mock(Cache.class);
CacheManager cacheManager = mock(CacheManager.class);
Connection conn = mock(Connection.class);
PreparedStatement ps = mock(PreparedStatement.class);
EntityTypes entityTypes = mock(EntityTypes.class);
ResultSet rs = mock(ResultSet.class);
IEntity entity = mock(IEntity.class);

// define behavior for mocked classes
// during initialization
applicationContextLocator
.when(() -> ApplicationContextLocator.getApplicationContext())
.thenReturn(context);
when(context.getBean("cacheManager", CacheManager.class)).thenReturn(cacheManager);
when(cacheManager.getCache(CACHE_NAME)).thenReturn(groupSearchCache);
when(cacheManager.getCache(PARENT_GROUP_BY_ENTITY_CACHE_NAME))
.thenReturn(parentGroupEntityCache);
when(cacheManager.getCache(PARENT_GROUP_BY_ENTTITY_GROUP_CACHE_NAME))
.thenReturn(parentGroupEntityGroupCache);
// during findParentGroups
when(entity.getKey()).thenReturn(cacheKey);
entityTypesLocator
.when(() -> EntityTypesLocator.getEntityTypes())
.thenReturn(entityTypes);
when(entityTypes.getEntityTypeFromID(anyInt())).thenReturn(null);
// during findParentGroupsForEntity
rdbmServices.when(() -> RDBMServices.getConnection()).thenReturn(conn);
when(conn.prepareStatement(anyString())).thenReturn(ps);
when(ps.executeQuery()).thenReturn(rs);
when(rs.next()).thenReturn(true, false);
// set to null in order to avoid more mocks in
// RDBMEntityGroupStore.instanceFromResultSet since we don't care about the results here
when(rs.getString(1)).thenReturn(null);
when(rs.getString(2)).thenReturn("ignore-me");
// value is ignored by entityTypes.getEntityTypeFromID() (above)
when(rs.getInt(3)).thenReturn(1);
when(rs.getString(4)).thenReturn("ignore-me");
when(rs.getString(5)).thenReturn("ignore-me");

Collection collection = new ArrayList();
Element el = new Element(cacheKey, collection.iterator());
when(parentGroupEntityCache.get(cacheKey)).thenReturn(null, el, el, el);

// for the purposes of this test, we don't care what is actually returned
// from the database; we're just confirming that the cache is being
// accessed the correct number of times
RDBMEntityGroupStore store = new RDBMEntityGroupStore();
store.findParentGroups(entity);
store.findParentGroups(entity);
store.findParentGroups(entity);
store.findParentGroups(entity);
verify(parentGroupEntityCache, times(4)).get(cacheKey);
verify(parentGroupEntityCache, times(1)).put(el);
}
}

@Test
public void testFindParentGroupsByIEntityGroup() throws Exception {
// This test confirms that calling findParentGroups(IEntityGroup) with the same key
// multiple times will check the cache each time but will only write
// to the cache one time.
//
// This test reflects the complexity of the RDBMEntityGroupStore class
// The RDBMEntityGroupStore class should really be two separate classes:
// a repository for reading from the database
// a service for processing the results.
//
// In addition, RDBMEntityGroupStore should be migrated to Spring annotations
// and bean management, which would eliminate the need for CacheManager
// and ApplicationContext
String cacheKey = "cache-key";
String serviceName = "service-name";
try (MockedStatic<ApplicationContextLocator> applicationContextLocator =
mockStatic(ApplicationContextLocator.class);
MockedStatic<RDBMServices> rdbmServices = mockStatic(RDBMServices.class);
MockedStatic<EntityTypesLocator> entityTypesLocator =
mockStatic(EntityTypesLocator.class)) {
// define all mocks
ApplicationContext context = mock(ApplicationContext.class);
Cache groupSearchCache = mock(Cache.class);
Cache parentGroupEntityCache = mock(Cache.class);
Cache parentGroupEntityGroupCache = mock(Cache.class);
CacheManager cacheManager = mock(CacheManager.class);
Name name = mock(Name.class);
Connection conn = mock(Connection.class);
PreparedStatement ps = mock(PreparedStatement.class);
EntityTypes entityTypes = mock(EntityTypes.class);
ResultSet rs = mock(ResultSet.class);
IEntityGroup entityGroup = mock(IEntityGroup.class);

// define behavior for mocked classes
// during initialization
applicationContextLocator
.when(() -> ApplicationContextLocator.getApplicationContext())
.thenReturn(context);
when(context.getBean("cacheManager", CacheManager.class)).thenReturn(cacheManager);
when(cacheManager.getCache(CACHE_NAME)).thenReturn(groupSearchCache);
when(cacheManager.getCache(PARENT_GROUP_BY_ENTITY_CACHE_NAME))
.thenReturn(parentGroupEntityCache);
when(cacheManager.getCache(PARENT_GROUP_BY_ENTTITY_GROUP_CACHE_NAME))
.thenReturn(parentGroupEntityGroupCache);
// during findParentGroups
when(entityGroup.getLocalKey()).thenReturn(cacheKey);
when(name.toString()).thenReturn(serviceName);
when(entityGroup.getServiceName()).thenReturn(name);
entityTypesLocator
.when(() -> EntityTypesLocator.getEntityTypes())
.thenReturn(entityTypes);
when(entityTypes.getEntityTypeFromID(anyInt())).thenReturn(null);
// during findParentGroupsForEntity
rdbmServices.when(() -> RDBMServices.getConnection()).thenReturn(conn);
when(conn.prepareStatement(anyString())).thenReturn(ps);
when(ps.executeQuery()).thenReturn(rs);
when(rs.next()).thenReturn(true, false);
// set to null in order to avoid more mocks in
// RDBMEntityGroupStore.instanceFromResultSet since we don't care about the results here
when(rs.getString(1)).thenReturn(null);
when(rs.getString(2)).thenReturn("ignore-me");
// value is ignored by entityTypes.getEntityTypeFromID() (above)
when(rs.getInt(3)).thenReturn(1);
when(rs.getString(4)).thenReturn("ignore-me");
when(rs.getString(5)).thenReturn("ignore-me");

Collection collection = new ArrayList();
Element el = new Element(cacheKey, collection.iterator());
when(parentGroupEntityGroupCache.get(cacheKey)).thenReturn(null, el, el, el);

// for the purposes of this test, we don't care what is actually returned
// from the database; we're just confirming that the cache is being
// accessed the correct number of times
RDBMEntityGroupStore store = new RDBMEntityGroupStore();
store.findParentGroups(entityGroup);
store.findParentGroups(entityGroup);
store.findParentGroups(entityGroup);
store.findParentGroups(entityGroup);
verify(parentGroupEntityGroupCache, times(4)).get(cacheKey);
verify(parentGroupEntityGroupCache, times(1)).put(el);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
mock-maker-inline
29 changes: 29 additions & 0 deletions uPortal-webapp/src/main/resources/properties/ehcache.xml
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@
eternal="false" maxElementsInMemory="50" overflowToDisk="false" diskPersistent="false"
timeToIdleSeconds="0" timeToLiveSeconds="28800" memoryStoreEvictionPolicy="LRU" statistics="true" />

<!--
| Caches Window State Collection -
| - Many-to-One join of StyleSheet and Window State
| - not replicated - represents local file system data
+-->
<cache name="org.apereo.portal.portlet.registry.PersistentPortletEntityWrapper.windowStates"
eternal="false" maxElementsInMemory="50" overflowToDisk="false" diskPersistent="false"
timeToIdleSeconds="0" timeToLiveSeconds="1800" memoryStoreEvictionPolicy="LRU" statistics="true" />

<!--
| Caches Skin Resource DocumentFragment instances, the XML chunk added to the <head>
| - 1 x skin
Expand Down Expand Up @@ -423,6 +432,26 @@
<cacheEventListenerFactory class="org.apereo.portal.utils.cache.SpringCacheEventListenerFactory" properties="beanName=insufficientSizeCacheEventListener" listenFor="local" />
</cache>

<!--
| Caches search results from findParentGroups() in RDBMEntityGroupStore.
| - 1 x search criteria
+-->
<cache name="org.apereo.portal.groups.RDBMEntityGroupStore.parentGroupEntity"
eternal="false" maxElementsInMemory="500" overflowToDisk="false" diskPersistent="false"
timeToIdleSeconds="0" timeToLiveSeconds="300" memoryStoreEvictionPolicy="LRU" statistics="true" >
<cacheEventListenerFactory class="org.apereo.portal.utils.cache.SpringCacheEventListenerFactory" properties="beanName=insufficientSizeCacheEventListener" listenFor="local" />
</cache>

<!--
| Caches search results from findParentGroups() in RDBMEntityGroupStore.
| - 1 x search criteria
+-->
<cache name="org.apereo.portal.groups.RDBMEntityGroupStore.parentGroupEntityGroup"
eternal="false" maxElementsInMemory="500" overflowToDisk="false" diskPersistent="false"
timeToIdleSeconds="0" timeToLiveSeconds="300" memoryStoreEvictionPolicy="LRU" statistics="true" >
<cacheEventListenerFactory class="org.apereo.portal.utils.cache.SpringCacheEventListenerFactory" properties="beanName=insufficientSizeCacheEventListener" listenFor="local" />
</cache>

<!--
| Caches IPermissionSet objects
| - 1 x per permissions owner (channel manager, user, ...)
Expand Down

0 comments on commit e8db822

Please sign in to comment.