Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Migrated to Confluence 5.3

...

Code Block
Refactored PersonDirNameFinder
Refactored PersonDirNameFinder
/**
 * Implementation of <code>IEntityNameFinder</code> for <code>IPersons</code> by 
 * looking up displayName from an <code>IPersonAttributeDao</code>.
 * @version Revision: 1.13++ 
 */
public class PersonDirNameFinder
        implements IEntityNameFinder {
    
    /**
     * Data Access Object backing this name finder.
     */
    private IPersonAttributeDao paDao;
    
    /** Our cache of entity names: */
    private Map names = Collections.synchronizedMap(new SoftHashMap());

    /**
     * Instantiate a PersonDirNameFinder backed by the given
     * IPersonAttributeDao.
     * @param pa DAO to back this PersonDirNameFinder
     */
    PersonDirNameFinder (IPersonAttributeDao pa) {
        this.paDao = pa;
    }


    public String getName (String key) {
        String name = (String) this.names.get(key);
        
        if (name == null) {
            // cached name not found, get name from underlying DAO.
            name = primGetName(key);
            // cache the name
            this.names.put(key, name);
        }
        return  name;
    }


    public java.util.Map getNames (java.lang.String[] keys) {
        Map selectedNames = new HashMap();
        for (int i = 0; i < keys.length; i++) {
            String name = getName(keys[i]);
            selectedNames.put(keys[i], name);
        }
        return  selectedNames;
    }


    public Class getType () {
        return  org.jasig.portal.security.IPerson.class;
    }

    /**
     * Actually lookup a user name using the underlying IPersonAttributeDao.
     * @param key - entity key which in this case is a unique identifier for a user
     * @return the display name for the identified user
     */
    private String primGetName (String key) {
        String name = key;
        Map userInfo = this.paDao.getUserAttributes(name);
        if (userInfo != null)
        {
            Object displayName = userInfo.get("displayName");
            String displayNameStr = "";
            if (displayName != null)
            {
                if (displayName instanceof java.util.List)
                {
                    List displayNameList = (List) displayName;
                    if (! displayNameList.isEmpty() )
                        { displayNameStr = (String)displayNameList.get(0); } 
                }
                else displayNameStr = (String)displayName;
        
                if (! displayNameStr.trim().equals("")) 
                    { name = displayNameStr; }
            }
        }
        return  name;
    }

    /**
     * Get a static singleton instance of this class backed by PersonDirectory.
     * @return singleton PersonDirNameFinder backed by PersonDirectory
     * @deprecated as of uP 2.5 instead use PersonDirNameFinderFactory
     */
    public static IEntityNameFinder singleton () {
        return new PersonDirNameFinderFactory().newFinder();
    }

    /**
     * Returns a String that represents the value of this object.
     * @return a string representation of the receiver
     */
    public String toString () {
        return  "PersonDirNameFinder backed by " + this.paDao;
    }
}

...

Dropped method primGetNames() because all this private method did was give us access to the private field private SoftHashMap names. The method was private and so was not providing an opportunity for anyone to extend this class and override the method to have us use some other Map.

Returning null Thread safety

This implementation of getName wasn't threadsafe() could return null: JIRA issue for fix for uP 2.4.3 JIRA issue for fix for uP 2.5

Code Block
titleNon-threadsafe getName() implementation that can return null
    /**
     * Given the key, returns the entity's name.
     * @param key java.lang.String
     */
    public String getName (String key) throws Exception {
        if (primGetNames().get(key) == null) {
            primGetNames().put(key, primGetName(key));
        }
        return  (String)primGetNames().get(key);
    }

...

By keeping a local reference to the displayName String, we make the implementation threadsafe:

Code Block
titleThreadsafe getName() implementation that won't return null
 public String getName (String key) {
        String name = (String) this.names.get(key);
        
        if (name == null) {
            // cached name not found, get name from underlying DAO.
            name = primGetName(key);
            // cache the name
            this.names.put(key, name);
        }
        return  name;
    }

However, we're still not threadsafe, because multiple threads can be accessing the instance SoftHashMap "name" while one or more threads are changing that map. So, when we construct our SoftHashMap, we wrap it in a Collections.synchronizedMap(). JIRA issue for fix in uPortal 2.4.3 JIRA issue for fix in uPortal 2.5

Exceptions

While the IEntityNameFinder interface entitles us to throw Exception, we don't actually need to throw any checked Exceptions. So the local methods here are not declared to throw exceptions. Anyone accessing us as a PersonDirNameFinder, rather than as an IEntityNameFinder won't have to deal with these exceptions.

Refactored factory

And here's a refactored version of the factory:

...