On Fri, Dec 19, 2008 at 1:45 AM, Yonik Seeley <[email protected]> wrote: > 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. That means normalize() would return "" for null? > > -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. >> >
-- --Noble Paul
