Skip to content

Commit

Permalink
SAK-25697 remove JLDAP caching code. If you want to cache your LDAP u…
Browse files Browse the repository at this point in the history 
…sers, do it in the central UserDirectoryService.callCache instead!

git-svn-id: https://source.sakaiproject.org/svn/providers/trunk@134278 66ffb92e-73f9-0310-93c1-f5514f145a0a
  • Loading branch information
@ottenhoff
ottenhoff committed Feb 8, 2014
1 parent 52f6189 commit e53fc01
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 594 deletions.
18 changes: 7 additions & 11 deletions providers/component/src/webapp/WEB-INF/jldap-beans.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
class="edu.amc.sakai.user.JLDAPDirectoryProvider" init-method="init"
destroy-method="destroy" singleton="true">

<property name="memoryService">
<ref bean="org.sakaiproject.memory.api.MemoryService"/>
</property>
<!-- Setting a MemoryService in JLDAP is deprecated!
Caching is done centrally in the UserDirectoryService.callCache -->

<!-- Required. Host name or address of your LDAP server -->
<property name="ldapHost">
Expand Down Expand Up @@ -97,16 +96,13 @@
</property -->

<!-- Setting a cacheTTL in jldap-beans.xml is longer supported!
If you are attempting to modify the JLDAP provider cache, you should use this format in sakai.properties:
memory.edu.amc.sakai.user.JLDAPDirectoryProvider.userCache=timeToLiveSeconds=3600,timeToIdleSeconds=0,maxElementsInMemory=5000
If you are attempting to cache your users for one hour instead of the default of 5 minutes,
set the central UserDirectoryService.callCache
memory.org.sakaiproject.user.api.UserDirectoryService.callCache=timeToLiveSeconds=3600
-->

<!-- Optional. Control case-sensitivity of cache keys (User.eid values).
Defaults to false. (Note that this is a departure from historical
behavior.) -->
<property name="caseSensitiveCacheKeys">
<value>false</value>
</property>
<!-- Setting caseSensitiveCacheKeys is no longer supported!
We assume that user jdoe is the same as user JDoe!! -->

<!-- Optional. Control the return value of
JLDAPDirectoryProvider.authenticateWithProviderFirst(String)
Expand Down
166 changes: 13 additions & 153 deletions providers/jldap/src/java/edu/amc/sakai/user/JLDAPDirectoryProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.sakaiproject.memory.api.Cache;
import org.sakaiproject.memory.api.MemoryService;
import org.sakaiproject.user.api.ExternalUserSearchUDP;
import org.sakaiproject.user.api.User;
import org.sakaiproject.user.api.UserDirectoryProvider;
Expand Down Expand Up @@ -92,8 +90,6 @@ public class JLDAPDirectoryProvider implements UserDirectoryProvider, LdapConnec
/** Default LDAP maximum number of objects to query for */
public static final int DEFAULT_BATCH_SIZE = 200;

public static final boolean DEFAULT_CASE_SENSITIVE_CACHE_KEYS = false;

public static final boolean DEFAULT_ALLOW_AUTHENTICATION = true;

public static final boolean DEFAULT_AUTHENTICATE_WITH_PROVIDER_FIRST = false;
Expand Down Expand Up @@ -170,17 +166,6 @@ public class JLDAPDirectoryProvider implements UserDirectoryProvider, LdapConnec
*/
private Map<String,String> attributeMappings;

private MemoryService memoryService;

/**
* Cache of {@link LdapUserData} objects, keyed by eid.
* {@link cacheTtl} controls TTL.
*
* TODO: This is a naive implementation: cache
* is completely isolated on each app node.
*/
private Cache userCache;

/** Handles LDAPConnection allocation */
private LdapConnectionManager ldapConnectionManager;

Expand All @@ -194,23 +179,19 @@ public class JLDAPDirectoryProvider implements UserDirectoryProvider, LdapConnec
/**
* Defaults to an anon-inner class which handles {@link LDAPEntry}(ies)
* by passing them to {@link #mapLdapEntryOntoUserData(LDAPEntry)}, the
* result of which is passed to {@link #cacheUserData(LdapUserData)}
* and returned;
* result of which is returned.
*/
protected LdapEntryMapper defaultLdapEntryMapper = new LdapEntryMapper() {

// doesn't update UserEdit in the off chance the search result actually
// yields multiple records
public Object mapLdapEntry(LDAPEntry searchResult, int resultNum) {
LdapUserData cacheRecord = mapLdapEntryOntoUserData(searchResult);
cacheUserData(cacheRecord);
return cacheRecord;
}

};

private boolean caseSensitiveCacheKeys = DEFAULT_CASE_SENSITIVE_CACHE_KEYS;

/**
* Flag for allowing/disallowing authentication on a global basis
*/
Expand Down Expand Up @@ -242,7 +223,7 @@ public void init()
if ( M_log.isDebugEnabled() ) {
M_log.debug("init()");
}
userCache = memoryService.newCache(getClass().getName()+".userCache");

// We don't want to allow people to break their config by setting the batch size to be more than the maxResultsSize.
if (batchSize > maxResultSize) {
batchSize = maxResultSize;
Expand Down Expand Up @@ -354,18 +335,6 @@ public void destroy()
M_log.debug("destroy()");
}

clearCache();
}

/**
* Resets the internal {@link LdapUserData} cache
*/
public void clearCache() {
if ( M_log.isDebugEnabled() ) {
M_log.debug("clearCache()");
}

userCache.clear();
}

/**
Expand Down Expand Up @@ -647,15 +616,8 @@ public void getUsers(Collection<UserEdit> users)
//proceed ahead with this (perhaps the final) iteration
//usersToSearchInLDAP needs to be processed unless empty
} else {
// Check the cache before sending the request to LDAP
LdapUserData cachedUserData = getCachedUserEntry(eid);
if ( cachedUserData == null ) {
usersToSearchInLDAP.put(eid, userEdit);
cnt++;
} else {
// populate userEdit with cached ldap data:
mapUserDataOntoUserEdit(cachedUserData, userEdit);
}
}

// We need to make sure this query isn't larger than maxQuerySize
Expand All @@ -673,9 +635,7 @@ public void getUsers(Collection<UserEdit> users)
if (StringUtils.isEmpty(ldapEid)) {
continue;
}
if (!(caseSensitiveCacheKeys)) {
ldapEid = ldapEid.toLowerCase();
}

UserEdit ue = usersToSearchInLDAP.get(ldapEid);
mapUserDataOntoUserEdit(ldapUserData, ue);
Expand Down Expand Up @@ -804,16 +764,6 @@ protected LdapUserData getUserByEid(String eid, LDAPConnection conn)
M_log.debug("getUserByEid(): [eid = " + eid + "]");
}

LdapUserData cachedUserData = getCachedUserEntry(eid);
boolean foundCachedUserData = cachedUserData != null;

if ( foundCachedUserData ) {
if ( M_log.isDebugEnabled() ) {
M_log.debug("getUserByEid(): found cached user [eid = " + eid + "]");
}
return cachedUserData;
}

if ( !(isSearchableEid(eid)) ) {
if ( M_log.isInfoEnabled() ) {
M_log.info("User EID not searchable (possibly blacklisted or otherwise syntactically invalid) [" + eid + "]");
Expand Down Expand Up @@ -1124,87 +1074,16 @@ protected void mapUserDataOntoUserEdit(LdapUserData userData, UserEdit userEdit)

if ( M_log.isDebugEnabled() ) {
// std. UserEdit impl has no meaningful toString() impl
M_log.debug("mapUserDataOntoUserEdit() [cache record = " + userData + "]");
M_log.debug("mapUserDataOntoUserEdit() [userData = " + userData + "]");
}

// delegate to the LdapAttributeMapper since it knows the most
// about how the LdapUserData instance was originally populated
ldapAttributeMapper.mapUserDataOntoUserEdit(userData, userEdit);

// This is not an entirely satisfactory solution, but it's important
// for all attribute mapping to respect this configuration (SAK-12705),
// so we centralized the logic rather than rely on swappable attribute
// mapping plugins. We decided to override the EID casing when mapping
// to UserEdits rather than when mapping to LdapUserDatas since we
// felt it was better to keep the caching of LDAP data and the mapping
// of that data to Sakai-consumable values as separate concerns.
//
// One wonders if a better solution might be to enforce case-sentivity
// rules where they matter, though, which is currenty in the UDS.
if ( !(caseSensitiveCacheKeys) ) {
userEdit.setEid(toCaseInsensitiveCacheKey(userData.getEid()));
}
}

/**
* Retrieve a user record from the cache.
*
* @param eid the cache key
* @return a user cache record, or null if a cache miss
*/
protected LdapUserData getCachedUserEntry(String eid) {

if ( M_log.isDebugEnabled() ) {
M_log.debug("getCachedUserEntry(): [eid = " + eid + "]");
}
if ( !(caseSensitiveCacheKeys) ) {
eid = toCaseInsensitiveCacheKey(eid);
}
LdapUserData cachedUserEntry = (LdapUserData) userCache.get(eid);

if (cachedUserEntry != null) {
if ( M_log.isDebugEnabled() ) {
M_log.debug("getCachedUserEntry(): [found entry = " + cachedUserEntry.toString() + "]");
}

return cachedUserEntry;
}

return null;
}

/**
* Add a {@link LdapUserData} object to the cache. Responsible
* for the setting the freshness timestamp.
*
* @param user the {@link LdapUserData} to add to the cache
*/
protected void cacheUserData(LdapUserData user){
String eid = user.getEid();

if ( eid == null ) {
throw new IllegalArgumentException("Attempted to cache a user record without an eid [UserData = " + user + "]");
}
user.setTimeStamp(System.currentTimeMillis());

if ( M_log.isDebugEnabled() ) {
M_log.debug("cacheUserData(): [user record = " + user + "]");
}

if ( !(caseSensitiveCacheKeys) ) {
eid = toCaseInsensitiveCacheKey(eid);
}

userCache.put(eid, user);
userEdit.setEid(StringUtils.lowerCase(userData.getEid()));
}

protected String toCaseInsensitiveCacheKey(String eid) {
if ( eid == null ) {
return null;
}
return eid.toLowerCase();
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -1520,31 +1399,12 @@ public void setLdapAttributeMapper(LdapAttributeMapper ldapAttributeMapper) {
}

/**
* Set the cache key case-sensitivity behavior. Defaults to
* {@link #DEFAULT_CASE_SENSITIVE_CACHE_KEYS}. At this writing,
* the cache is keyed exclusively by <code>User.eid</code> values.
*
* @see #cacheUserData(LdapUserData)
* @see #getCachedUserEntry(String)
* @see #defaultLdapEntryMapper
* Used to set the cache key case-sensitivity behavior.
* @param caseSensitive
* @deprecated
*/
public void setCaseSensitiveCacheKeys(boolean caseSensitive) {
this.caseSensitiveCacheKeys = caseSensitive;
}

/**
* Access the cache key case-sensitivity behavior. Defaults to
* {@link #DEFAULT_CASE_SENSITIVE_CACHE_KEYS}. At this writing,
* the cache is keyed exclusively by <code>User.eid</code> values.
*
* @see #cacheUserData(LdapUserData)
* @see #getCachedUserEntry(String)
* @see #defaultLdapEntryMapper
* @return boolean
*/
public boolean isCaseSensitiveCacheKeys() {
return caseSensitiveCacheKeys;
M_log.warn("DEPRECATION WARNING: caseSensitiveCacheKeys is deprecated. Please remove it from your jldap-beans.xml configuration.");
}

/**
Expand Down Expand Up @@ -1672,12 +1532,12 @@ public void setSearchScope(int searchScope) throws IllegalArgumentException {
}
}

public MemoryService getMemoryService() {
return memoryService;
}

public void setMemoryService(MemoryService memoryService) {
this.memoryService = memoryService;
/**
* User caching is done centrally in the UserDirectoryService.callCache
* @deprecated
**/
public void setMemoryService(org.sakaiproject.memory.api.MemoryService ignore) {
M_log.warn("DEPRECATION WARNING: memoryService is deprecated. Please remove it from your jldap-beans.xml configuration.");
}

/**
Expand Down
22 changes: 1 addition & 21 deletions providers/jldap/src/java/edu/amc/sakai/user/LdapUserData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ public class LdapUserData
private String type;

private Properties properties;

private long timeStamp;


/**
Expand Down Expand Up @@ -147,23 +145,6 @@ public void setType(String type)
this.type = type;
}

/**
* @return Returns the timeStamp.
*/
public long getTimeStamp()
{
return timeStamp;
}

/**
* @param timeStamp
* The timeStamp to set.
*/
public void setTimeStamp(long timeStamp)
{
this.timeStamp = timeStamp;
}

/**
* @return Returns the user's properties
*/
Expand Down Expand Up @@ -204,8 +185,7 @@ public String toString() {
.append("lastName",lastName)
.append("email",email)
.append("type",type)
.append("timeStamp", timeStamp)
.append("properties",properties)
.toString();
}
}
}
3 changes: 1 addition & 2 deletions providers/jldap/src/java/edu/amc/sakai/user/SimpleLdapAttributeMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,7 @@ protected String mapLdapEntryToSakaiUserType(LDAPEntry ldapEntry) {
public void mapUserDataOntoUserEdit(LdapUserData userData, UserEdit userEdit) {

if ( M_log.isDebugEnabled() ) {
M_log.debug("mapUserDataOntoUserEdit(): [cache record = " +
userData + "]");
M_log.debug("mapUserDataOntoUserEdit(): [userData = " + userData + "]");
}

userEdit.setEid(userData.getEid());
Expand Down
Loading

0 comments on commit e53fc01

Please sign in to comment.