Well there are several bugs, resulting from the over-aggressive
normalization combined with normalizing *always* despite this comment:

      // Only normalize factory names

So consider the case someone has
<similarity class="org.apache.lucene.search.similarities.BM25Similarity"/>

which is allowed (it uses the anonymous factory). In this case its
bogusly normalized to "solr.BM25Similarity" which is invalid and won't
be loaded by IndexSchema, since it only looks for solr.xxxx in
org.apache.solr.search.similarities.

I think a patch like the following is a good start, but we should
review the other uses of the same code-dup'ed function in IndexSchema
and ensure there are not similar bugs:

I'm sorry if i came off terse or as a haiku, its not a big deal, I
just want it to work correctly.

Index: solr/core/src/java/org/apache/solr/schema/SimilarityFactory.java
===================================================================
--- solr/core/src/java/org/apache/solr/schema/SimilarityFactory.java    
(revision
1460952)
+++ solr/core/src/java/org/apache/solr/schema/SimilarityFactory.java    (working
copy)
@@ -51,9 +51,9 @@
   public abstract Similarity getSimilarity();


-  private static String normalizeSPIname(String fullyQualifiedName) {
-    if (fullyQualifiedName.startsWith("org.apache.lucene.") ||
fullyQualifiedName.startsWith("org.apache.solr.")) {
-      return "solr" +
fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf('.'));
+  private static String normalizeName(String fullyQualifiedName) {
+    if (fullyQualifiedName.startsWith("org.apache.solr.search.similarities."))
{
+      return "solr" +
fullyQualifiedName.substring("org.apache.solr.search.similarities".length());
     }
     return fullyQualifiedName;
   }
@@ -66,10 +66,10 @@
       className = getSimilarity().getClass().getName();
     } else {
       // Only normalize factory names
-      className = normalizeSPIname(className);
+      className = normalizeName(className);
     }
     SimpleOrderedMap<Object> props = new SimpleOrderedMap<Object>();
-    props.add(CLASS_NAME, normalizeSPIname(className));
+    props.add(CLASS_NAME, className);
     if (null != params) {
       Iterator<String> iter = params.getParameterNamesIterator();
       while (iter.hasNext()) {


On Mon, Mar 25, 2013 at 12:04 PM, Steve Rowe <[email protected]> wrote:
> Robert,
>
> Would you mind responding in some form other than haiku?
>
> What's wrong to do?
>
> What should be done?
>
> Steve
>
> On Mar 25, 2013, at 1:28 PM, Robert Muir <[email protected]> wrote:
>
>> Right... This is wrong to do though.
>>
>> On Mar 25, 2013 6:00 AM, "Steve Rowe" <[email protected]> wrote:
>>
>> On Mar 25, 2013, at 3:51 AM, Robert Muir <[email protected]> wrote:
>> > But this 'normalize' is wrongt: like map org.apache.lucene.xxx, or
>> > other packages of org.apache.solr.xxx to solr.xxx.
>>
>> No, it maps o.a.(l|s).what.ev.er.xxx to solr.xxx.
>>
>> Here's the code again:
>>
>> -----
>> private static String normalizeSPIname(String fullyQualifiedName) {
>>   if (fullyQualifiedName.startsWith("org.apache.lucene.") || 
>> fullyQualifiedName.startsWith("org.apache.solr.")) {
>>     return "solr" + 
>> fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf('.'));
>>   }
>>   return fullyQualifiedName;
>> }
>> -----
>>
>> See the .lastIndexOf('.') part?
>>
>> Steve
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to