Re: [Pki-devel] [PATCH] pki-cfu-0009-TPS-Token-Profile-Resolver-Plugin-Framework-Ticket-4.patch

Jack, Thanks for the review comments. Attached please find the new patch that addressed your comments, both on-line and off-line. Christina On 05/30/2014 05:06 PM, John Magne wrote: > Couple of initial comments on this after looking over it: > > 1. > > > Your TokenProfileParams class has this hash map. > > private HashMap<String, Object> content = new HashMap<String, Object>(); > > Then when we add items and get items from this thing, we have to do > some fancy > footwork to account for the fact that the "Object" part is holding > either a String > value or an int value. > > My opinion after doing some research is that this is kind of awkward. > > A couple of possible solutions: > > 1. Have the HashMap hold <String,String>. > > When we want an int we just convert back and forth from string. > This is done often when processing http name value pairs and such. > > 2. Instead of relying upon the "get" method of the hash map to > return > encoded values, maybe have the parent object "TokenProfileParams" > have two > separate methods getString() and getInt(). Each one would take the > key name > and output the proper value, int or String > > > 2. We have the following abstract base class: > > public abstract class BaseTokenProfileResolver > > > The abstract class sounds like a good way to have some base > functionality to be used by derived classes. > I also believe that the idea is to declare some abstract methods that > forces derived classes to implement > these methods. > > In this case it sounds like the following method would be a good > candidate for abstract: > > public String getTokenType(TokenProfileParams pPram); > > > > > > > ----- Original Message ----- >> From: "Christina Fu" <cfu(a)redhat.com&gt; >> To: pki-devel(a)redhat.com >> Sent: Friday, May 30, 2014 3:07:56 PM >> Subject: [Pki-devel] [PATCH] >> pki-cfu-0009-TPS-Token-Profile-Resolver-Plugin-Framework-Ticket-4.patch >> >> This patch implements a Token Profile Resolver Plugin Framework for >> TPS. It allows a site to implement its own token profile resolver. >> >> The TokenProfileResolverManager initializes the token Resolver plugin >> framework from the following configuration files: >> - <instance patch>/conf/registry.cfg >> - <instance patch>/conf/CS.cfg >> >> The existing mapping method "getTokenType" was extracted into the >> default plugin code: MappingTokenProfileResolver. All resolver plugin >> codes will extend from BaseTokenProfileResolver. >> >> The most visible change from sites running previous versions of TPS is >> that the mapping related config params >> e.g. op.format.mapping.xxx >> have now being extracted into token resolver instance params >> e.g. tokenProfileResolver.formatMappingResolver.mapping.xxxx >> where "class_id" defines the plugin implementation name defined >> in the >> registry.cfg >> and op.format.tokenProfileResolver points to the defined resolver >> instance >> e.g. op.format.tokenProfileResolver=formatMappingResolver >> >> A separate ticket will be filed to provide actual plugin writing >> instruction as well as a sample plugin. >> >> Finally, even though this patch does not directly address >> https://fedorahosted.org/pki/ticket/447 RFE: Mapping tokens to >> tokentype, where the request was to figure out the tokenType (Token >> profile) by uid instead of the current mapping method, it provides the >> means for one to write any customized plugins. >> >> thanks, >> Christina >> >> _______________________________________________ >> Pki-devel mailing list >> Pki-devel(a)redhat.com >> https://www.redhat.com/mailman/listinfo/pki-devel _______________________________________________ Pki-devel mailing list Pki-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel