LUCENE-4055: clear some nocommits

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/branches/lucene4055@1341513 13f79535-47bb-0310-9956-ffa450edef68

lucene/core/src/java/org/apache/lucene/codecs/Codec.java

@@ -45,12 +45,13 @@ public abstract class Codec implements NamedSPILoader.NamedSPI {
   private final String name;
 
   public Codec(String name) {
+    NamedSPILoader.checkServiceName(name);
     this.name = name;
   }
   
   /** Returns this codec's name */
   @Override
-  public String getName() {
+  public final String getName() {
     return name;
   }
   

lucene/core/src/java/org/apache/lucene/codecs/PostingsFormat.java

@@ -20,7 +20,6 @@ package org.apache.lucene.codecs;
 import java.io.IOException;
 import java.util.Set;
 
-import org.apache.lucene.index.SegmentInfo;
 import org.apache.lucene.index.SegmentWriteState;
 import org.apache.lucene.index.SegmentReadState;
 import org.apache.lucene.util.NamedSPILoader;
@@ -40,14 +39,12 @@ public abstract class PostingsFormat implements NamedSPILoader.NamedSPI {
   private final String name;
   
   protected PostingsFormat(String name) {
-    // nocommit: check that name is a-zA-Z0-9 and < some reasonable length
-    // also fix this for Codec
-    // also make NamedSPILoader's map case-insensitive (like Charset)
+    NamedSPILoader.checkServiceName(name);
     this.name = name;
   }
 
   @Override
-  public String getName() {
+  public final String getName() {
     return name;
   }
   

lucene/core/src/java/org/apache/lucene/codecs/perfield/PerFieldPostingsFormat.java

@@ -96,9 +96,11 @@ public abstract class PerFieldPostingsFormat extends PostingsFormat {
         formats.put(format, consumer);
       }
 
-      // nocommit we should only provide the "slice" of FIS
+      // TODO: we should only provide the "slice" of FIS
       // that this PF actually sees ... then stuff like
       // .hasProx could work correctly?
+      // NOTE: .hasProx is already broken in the same way for the non-perfield case,
+      // if there is a fieldinfo with prox that has no postings, you get a 0 byte file.
       return consumer.addField(field);
     }
 

lucene/core/src/java/org/apache/lucene/util/NamedSPILoader.java

@@ -28,6 +28,7 @@ import java.util.ServiceLoader;
  * Helper class for loading named SPIs from classpath (e.g. Codec, PostingsFormat).
  * @lucene.internal
  */
+// TODO: would be nice to have case insensitive lookups.
 public final class NamedSPILoader<S extends NamedSPILoader.NamedSPI> implements Iterable<S> {
 
   private final Map<String,S> services;
@@ -51,6 +52,7 @@ public final class NamedSPILoader<S extends NamedSPILoader.NamedSPI> implements
       // this allows to place services before others in classpath to make 
       // them used instead of others
       if (!services.containsKey(name)) {
+        assert checkServiceName(name);
         services.put(name, service);
       }
     }
@@ -58,6 +60,37 @@ public final class NamedSPILoader<S extends NamedSPILoader.NamedSPI> implements
     this.services = Collections.unmodifiableMap(services);
   }
   
+  /**
+   * Validates that a service name meets the requirements of {@link NamedSPI}
+   */
+  public static boolean checkServiceName(String name) {
+    // based on harmony charset.java
+    if (name.length() >= 128) {
+      throw new IllegalArgumentException("Illegal service name: '" + name + "' is too long (must be < 128 chars).");
+    }
+    for (int i = 0; i < name.length(); i++) {
+      char c = name.charAt(i);
+      if (!isLetter(c) && !isDigit(c)) {
+        throw new IllegalArgumentException("Illegal service name: '" + name + "' must be simple ascii alphanumeric.");
+      }
+    }
+    return true;
+  }
+  
+  /*
+   * Checks whether a character is a letter (ascii) which are defined in the spec.
+   */
+  private static boolean isLetter(char c) {
+      return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+  }
+
+  /*
+   * Checks whether a character is a digit (ascii) which are defined in the spec.
+   */
+  private static boolean isDigit(char c) {
+      return ('0' <= c && c <= '9');
+  }
+  
   public S lookup(String name) {
     final S service = services.get(name);
     if (service != null) return service;
@@ -76,6 +109,8 @@ public final class NamedSPILoader<S extends NamedSPILoader.NamedSPI> implements
   
   /**
    * Interface to support {@link NamedSPILoader#lookup(String)} by name.
+   * <p>
+   * Names must be all ascii alphanumeric, and less than 128 characters in length.
    */
   public static interface NamedSPI {
     String getName();