What about just removing the offending "if(handlerName == null) return null;" That should give us acceptable back compatibility since "" and null were always meant to be the default.
-Yonik On Thu, Dec 18, 2008 at 6:43 AM, Shalin Shekhar Mangar <[email protected]> wrote: > Ok, so let us fix the root of the problem. > > 1. Disallow request handlers to be registered or looked up with null -- > throw NullPointerException? > 2. Remove the change in normalize > 3. Change internal Solr code to make sure we are never registering or > looking up with null key > > Is that fine? > > On Thu, Dec 18, 2008 at 4:38 PM, Ryan McKinley <[email protected]> wrote: > >> Right, but what about this: >> >> if(handlerName == null) return null; >> >> that returns null rather then the default! >> >> >> >> On Dec 18, 2008, at 6:01 AM, Noble Paul നോബിള് नोब्ळ् wrote: >> >> The idea is to make null and "" equivalent . COncurrentHashMap cannot >>> take null keys >>> >>> On Thu, Dec 18, 2008 at 2:17 PM, Chris Hostetter >>> <[email protected]> wrote: >>> >>>> >>>> Uh... wait a minute, what is "if(handlerName == null) return null;" doing >>>> in the "register" method? that's not the same semantics as before. >>>> >>>> and why are there redundent null checks in normalize? >>>> >>>> if the intention is to make null equivilent to "" let's make it work >>>> correctly (and mention it in the javadocs) >>>> >>>> : Date: Wed, 17 Dec 2008 13:12:40 -0000 >>>> : From: [email protected] >>>> : Reply-To: [email protected] >>>> : To: [email protected] >>>> : Subject: svn commit: r727372 - >>>> : >>>> /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java >>>> : >>>> : Author: shalin >>>> : Date: Wed Dec 17 05:12:40 2008 >>>> : New Revision: 727372 >>>> : >>>> : URL: http://svn.apache.org/viewvc?rev=727372&view=rev >>>> : Log: >>>> : SOLR-917 -- Change RequestHandlers#handlers from a synchronizedMap to a >>>> ConcurrentHashMap >>>> : >>>> : Modified: >>>> : >>>> lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java >>>> : >>>> : Modified: >>>> lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java >>>> : URL: >>>> http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java?rev=727372&r1=727371&r2=727372&view=diff >>>> : >>>> ============================================================================== >>>> : --- >>>> lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java >>>> (original) >>>> : +++ >>>> lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java Wed >>>> Dec >>>> 17 05:12:40 2008 >>>> : @@ -21,6 +21,8 @@ >>>> : import java.util.Collections; >>>> : import java.util.HashMap; >>>> : import java.util.Map; >>>> : +import java.util.concurrent.ConcurrentHashMap; >>>> : + >>>> : import org.slf4j.Logger; >>>> : import org.slf4j.LoggerFactory; >>>> : >>>> : @@ -50,8 +52,8 @@ >>>> : protected final SolrCore core; >>>> : // Use a synchronized map - since the handlers can be changed at >>>> runtime, >>>> : // the map implementation should be thread safe >>>> : - private final Map<String, SolrRequestHandler> handlers = >>>> Collections.synchronizedMap( >>>> : - new HashMap<String,SolrRequestHandler>() ); >>>> : + private final Map<String, SolrRequestHandler> handlers = >>>> : + new ConcurrentHashMap<String,SolrRequestHandler>() ; >>>> : >>>> : /** >>>> : * Trim the trailing '/' if its there. >>>> : @@ -64,6 +66,7 @@ >>>> : */ >>>> : private static String normalize( String p ) >>>> : { >>>> : + if(p == null) return ""; >>>> : if( p != null && p.endsWith( "/" ) && p.length() > 1 ) >>>> : return p.substring( 0, p.length()-1 ); >>>> : >>>> : @@ -90,6 +93,7 @@ >>>> : * @return the previous handler at the given path or null >>>> : */ >>>> : public SolrRequestHandler register( String handlerName, >>>> SolrRequestHandler handler ) { >>>> : + if(handlerName == null) return null; >>>> : String norm = normalize( handlerName ); >>>> : if( handler == null ) { >>>> : return handlers.remove( norm ); >>>> : @@ -175,7 +179,6 @@ >>>> : register(RequestHandlers.DEFAULT_HANDLER_NAME, >>>> defaultHandler); >>>> : } >>>> : } >>>> : - register(null, defaultHandler); >>>> : register("", defaultHandler); >>>> : } >>>> : >>>> : >>>> : >>>> >>>> >>>> >>>> -Hoss >>>> >>>> >>>> >>> >>> >>> -- >>> --Noble Paul >>> >> >> > > > -- > Regards, > Shalin Shekhar Mangar. >
