Skip to content

Commit

Permalink
UP-5003: Entries in the 'org.apereo.portal.groups.GroupMemberImpl.par…
Browse files Browse the repository at this point in the history
…entGroups' cache are not purged when a user authenticates
  • Loading branch information
drewwills committed Mar 8, 2018
1 parent 70b1ffd commit 1d677b9
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
/**
* A key and type that uniquely identify a portal entity.
*
* <p>In the case of users, the key will be the username and the type will be {@link
* org.apereo.portal.security.IPerson}.
*
* @see IBasicEntity
*/
public class EntityIdentifier implements Serializable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public boolean deepContains(IGroupMember gm) throws GroupsException {
boolean found = false;
Iterator<IEntityGroup> it = getMemberGroups();
while (it.hasNext() && !found) {
IEntityGroup group = (IEntityGroup) it.next();
IEntityGroup group = it.next();
if (group != null) {
found = group.deepContains(gm);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@
import net.sf.ehcache.CacheManager;
import net.sf.ehcache.Element;
import org.apereo.portal.EntityIdentifier;
import org.apereo.portal.security.IPerson;
import org.apereo.portal.services.GroupService;
import org.apereo.portal.spring.locator.ApplicationContextLocator;
import org.apereo.portal.spring.locator.EntityTypesLocator;
import org.apereo.portal.utils.cache.CacheKey;
import org.apereo.portal.utils.cache.UsernameTaggedCacheEntryPurger;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.ApplicationContext;
Expand Down Expand Up @@ -68,7 +71,7 @@ public GroupMemberImpl(EntityIdentifier newEntityIdentifier) throws GroupsExcept
*/
@Override
public Set<IEntityGroup> getAncestorGroups() throws GroupsException {
return primGetAncestorGroups(this, new HashSet<IEntityGroup>());
return primGetAncestorGroups(this, new HashSet<>());
}

/**
Expand Down Expand Up @@ -129,9 +132,6 @@ public EntityIdentifier getUnderlyingEntityIdentifier() {
/**
* Answers if this <code>IGroupMember</code> is, recursively, a member of <code>IGroupMember
* </code> gm.
*
* @return boolean
* @param gm org.apereo.portal.groups.IGroupMember
*/
@Override
public boolean isDeepMemberOf(IEntityGroup group) throws GroupsException {
Expand All @@ -143,17 +143,13 @@ public boolean isDeepMemberOf(IEntityGroup group) throws GroupsException {
public boolean isGroup() {
return false;
}

/** @return boolean. */
protected boolean isKnownEntityType(Class anEntityType) throws GroupsException {
return (EntityTypesLocator.getEntityTypes().getEntityIDFromType(anEntityType) != null);
}

/**
* Answers if this <code>IGroupMember</code> is a member of <code>IGroupMember</code> gm.
*
* @param gm org.apereo.portal.groups.IGroupMember
* @return boolean
*/
/** Answers if this <code>IGroupMember</code> is a member of <code>IGroupMember</code> gm. */
@Override
public boolean isMemberOf(IEntityGroup group) throws GroupsException {
return getParentGroups().contains(group);
Expand Down Expand Up @@ -216,4 +212,16 @@ protected void invalidateInParentGroupsCache(Set<IGroupMember> members) {
parentGroupsCache.remove(member.getEntityIdentifier());
}
}

protected CacheKey getCacheKey(EntityIdentifier entityIdentifier) {
// Use tagged keys for users (only) so the cache entries will be dropped when they
// authenticate
return IPerson.class.equals(entityIdentifier.getType())
? CacheKey.buildTagged(
getClass().getName(),
UsernameTaggedCacheEntryPurger.createCacheEntryTag(
entityIdentifier.getKey()),
entityIdentifier)
: CacheKey.build(getClass().getName(), entityIdentifier);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public AdHocGroupTester(IPersonAttributesGroupTestDefinition definition) {
/*
* At some point, a person is being tested for group membership. During that test, the thread hits an ad hoc group
* tester. When that tester calls isDeepMemberOf, a test for group membership is triggered. During this call stack,
* the second call the the ad hoc group tester returns false. Assuming the group hierarchy is not itself recursive
* the second call to the ad hoc group tester returns false. Assuming the group hierarchy is not itself recursive
* for the group containing the ad hoc group test, the test returns a usable value.
*
* If there is no caching and the second person object only exists for the recursive call, then the implementation
Expand All @@ -93,7 +93,7 @@ public boolean test(IPerson person) {
IGroupMember gmPerson = findPersonAsGroupMember(person);
if (currentTests.getQuiet(personHash) != null) {
logger.debug(
"Returning from test() for {} due to recusion for person = {}",
"Returning from test() for {} due to recursion for person = {}",
personHash,
person.toString());
return false; // stop recursing
Expand All @@ -103,8 +103,14 @@ public boolean test(IPerson person) {
// method that potentially recurs
boolean isPersonGroupMember = gmPerson.isDeepMemberOf(entityGroup);
currentTests.remove(personHash);
logger.debug("Returning from test() for {}", personHash);
return isPersonGroupMember ^ isNotTest;
final boolean rslt = isPersonGroupMember ^ isNotTest;
logger.debug(
"Returning '{}' from test() for '{}' {} a (deep) member of '{}'",
rslt,
person.getUserName(),
isNotTest ? "is not" : "is",
groupName);
return rslt;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private void checkKeyMap() {

public static <K extends Serializable, V extends Serializable> CacheKeyBuilder<K, V> builder(
String source) {
return new CacheKeyBuilder<K, V>(source);
return new CacheKeyBuilder<>(source);
}

public static CacheKey build(String source, Serializable... key) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class UsernameTaggedCacheEntryPurger implements ApplicationListener<Logou
public static final String TAG_TYPE = "username";

public static CacheEntryTag createCacheEntryTag(String username) {
return new SimpleCacheEntryTag<String>(TAG_TYPE, username);
return new SimpleCacheEntryTag<>(TAG_TYPE, username);
}

private TaggedCacheEntryPurger taggedCacheEntryPurger;
Expand Down
1 change: 1 addition & 0 deletions uPortal-webapp/src/main/resources/properties/ehcache.xml
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@
<cache name="org.apereo.portal.groups.GroupMemberImpl.parentGroups"
eternal="false" maxElementsInMemory="5000" overflowToDisk="false" diskPersistent="false"
timeToIdleSeconds="0" timeToLiveSeconds="300" memoryStoreEvictionPolicy="LRU" statistics="true" >
<cacheEventListenerFactory class="org.apereo.portal.utils.cache.SpringCacheEventListenerFactory" properties="beanName=tagTrackingCacheEventListener" listenFor="local" />
<cacheEventListenerFactory
class="net.sf.ehcache.distribution.jgroups.JGroupsCacheReplicatorFactory"
properties="replicateAsynchronously=true,
Expand Down

0 comments on commit 1d677b9

Please sign in to comment.