You are viewing a plain text version of this content. The canonical link for it is here.
Posted to solr-dev@lucene.apache.org by Chris Hostetter <ho...@fucit.org> on 2008/12/18 09:47:46 UTC
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
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: shalin@apache.org
: Reply-To: solr-dev@lucene.apache.org
: To: solr-commits@lucene.apache.org
: 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
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Chris Hostetter <ho...@fucit.org>.
>> 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.
strictly speaking that's still not equivilent to the previous behavior,
because in the past people could call core.getRequestHandlers() and then
do a lookup by null *OR* "" to get the default handler -- now only "" will
work.
but i'm guessing we can get away with it.
Committed revision 727934.
-Hoss
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Noble Paul നോബിള് नोब्ळ् <no...@gmail.com>.
Perfect . That is what I had in mind. thanks
On Fri, Dec 19, 2008 at 10:16 AM, Yonik Seeley <ys...@gmail.com> wrote:
> On Thu, Dec 18, 2008 at 11:31 PM, Noble Paul നോബിള് नोब्ळ्
> <no...@gmail.com> wrote:
>> On Fri, Dec 19, 2008 at 1:45 AM, Yonik Seeley <ys...@gmail.com> 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?
>
> Yes, as it does right now... it doesn't seem like any other changes
> are needed except removing the "if(handlerName == null) return null;"
>
> -Yonik
>
--
--Noble Paul
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Yonik Seeley <ys...@gmail.com>.
On Thu, Dec 18, 2008 at 11:31 PM, Noble Paul നോബിള് नोब्ळ्
<no...@gmail.com> wrote:
> On Fri, Dec 19, 2008 at 1:45 AM, Yonik Seeley <ys...@gmail.com> 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?
Yes, as it does right now... it doesn't seem like any other changes
are needed except removing the "if(handlerName == null) return null;"
-Yonik
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Noble Paul നോബിള് नोब्ळ् <no...@gmail.com>.
On Fri, Dec 19, 2008 at 1:45 AM, Yonik Seeley <ys...@gmail.com> 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
> <sh...@gmail.com> 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 <ry...@gmail.com> 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
>>>> <ho...@fucit.org> 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: shalin@apache.org
>>>>> : Reply-To: solr-dev@lucene.apache.org
>>>>> : To: solr-commits@lucene.apache.org
>>>>> : 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
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Yonik Seeley <ys...@gmail.com>.
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
<sh...@gmail.com> 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 <ry...@gmail.com> 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
>>> <ho...@fucit.org> 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: shalin@apache.org
>>>> : Reply-To: solr-dev@lucene.apache.org
>>>> : To: solr-commits@lucene.apache.org
>>>> : 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.
>
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Shalin Shekhar Mangar <sh...@gmail.com>.
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 <ry...@gmail.com> 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
>> <ho...@fucit.org> 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: shalin@apache.org
>>> : Reply-To: solr-dev@lucene.apache.org
>>> : To: solr-commits@lucene.apache.org
>>> : 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.
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Ryan McKinley <ry...@gmail.com>.
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
> <ho...@fucit.org> 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: shalin@apache.org
>> : Reply-To: solr-dev@lucene.apache.org
>> : To: solr-commits@lucene.apache.org
>> : 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
Re: svn commit: r727372 - /lucene/solr/trunk/src/java/org/apache/solr/core/RequestHandlers.java
Posted by Noble Paul നോബിള് नोब्ळ् <no...@gmail.com>.
The idea is to make null and "" equivalent . COncurrentHashMap cannot
take null keys
On Thu, Dec 18, 2008 at 2:17 PM, Chris Hostetter
<ho...@fucit.org> 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: shalin@apache.org
> : Reply-To: solr-dev@lucene.apache.org
> : To: solr-commits@lucene.apache.org
> : 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