Changes after review (1)

-- Renamed to ElasticsearchSynonymParser
-- Added explanation for ElasticsearchSynonymParser::add method
-- Changed ElasticsearchSynonymParser::logger instance to static

Author: sohaibiftikhar

server/src/main/java/org/elasticsearch/index/analysis/ElasticSynonymParser.java renamed to server/src/main/java/org/elasticsearch/index/analysis/ElasticsearchSynonymParser.java

@@ -28,22 +28,28 @@
 
 import java.io.IOException;
 
-public class ElasticSynonymParser extends SolrSynonymParser {
+public class ElasticsearchSynonymParser extends SolrSynonymParser {
 
     private final boolean lenient;
-    private final Logger logger;
+    private static final Logger logger =
+        Loggers.getLogger(ElasticsearchSynonymParser.class, "ElasticsearchSynonymParser");
 
-    public ElasticSynonymParser(boolean dedup, boolean expand, boolean lenient, Analyzer analyzer) {
+    public ElasticsearchSynonymParser(boolean dedup, boolean expand, boolean lenient, Analyzer analyzer) {
         super(dedup, expand, analyzer);
         this.lenient = lenient;
-        logger = Loggers.getLogger(getClass(), "ElasticSynonymParser");
     }
 
     @Override
     public void add(CharsRef input, CharsRef output, boolean includeOrig) {
+        // This condition follows up on the overridden analyze method. In case lenient was set to true and there was an
+        // exception during super.analyze we return a zero-length CharsRef for that word which caused an exception. When
+        // the synonym mappings for the words are added using the add method we skip the ones that were left empty by
+        // analyze i.e., in the case when lenient is set we only add those combinations which are non-zero-length. The
+        // else would happen only in the case when the input or output is empty and lenient is set, in which case we
+        // quietly ignore it. For more details on the control-flow see SolrSynonymParser::addInternal.
         if (lenient == false || (input.length > 0 && output.length > 0)) {
             super.add(input, output, includeOrig);
-        } // the else would happen only in the case for lenient in which case we quietly ignore it
+        }
     }
 
     @Override

server/src/main/java/org/elasticsearch/index/analysis/SynonymGraphTokenFilterFactory.java

@@ -60,8 +60,8 @@ public Factory(String name, final Analyzer analyzerForParseSynonym, Reader rules
                     parser = new WordnetSynonymParser(true, expand, analyzerForParseSynonym);
                     ((WordnetSynonymParser) parser).parse(rulesReader);
                 } else {
-                    parser = new ElasticSynonymParser(true, expand, lenient, analyzerForParseSynonym);
-                    ((ElasticSynonymParser) parser).parse(rulesReader);
+                    parser = new ElasticsearchSynonymParser(true, expand, lenient, analyzerForParseSynonym);
+                    ((ElasticsearchSynonymParser) parser).parse(rulesReader);
                 }
                 synonymMap = parser.build();
             } catch (Exception e) {

server/src/main/java/org/elasticsearch/index/analysis/SynonymTokenFilterFactory.java

@@ -97,8 +97,8 @@ public Factory(String name, Analyzer analyzerForParseSynonym, Reader rulesReader
                     parser = new WordnetSynonymParser(true, expand, analyzerForParseSynonym);
                     ((WordnetSynonymParser) parser).parse(rulesReader);
                 } else {
-                    parser = new ElasticSynonymParser(true, expand, lenient, analyzerForParseSynonym);
-                    ((ElasticSynonymParser) parser).parse(rulesReader);
+                    parser = new ElasticsearchSynonymParser(true, expand, lenient, analyzerForParseSynonym);
+                    ((ElasticsearchSynonymParser) parser).parse(rulesReader);
                 }
                 synonymMap = parser.build();
             } catch (Exception e) {

server/src/test/java/org/elasticsearch/index/analysis/ElasticSynonymParserTests.java renamed to server/src/test/java/org/elasticsearch/index/analysis/ElasticsearchSynonymParserTests.java

@@ -33,10 +33,10 @@
 
 import static org.hamcrest.Matchers.containsString;
 
-public class ElasticSynonymParserTests extends ESTokenStreamTestCase {
+public class ElasticsearchSynonymParserTests extends ESTokenStreamTestCase {
 
     public void testLenientParser() throws IOException, ParseException {
-        ElasticSynonymParser parser = new ElasticSynonymParser(true, false, true, new StandardAnalyzer());
+        ElasticsearchSynonymParser parser = new ElasticsearchSynonymParser(true, false, true, new StandardAnalyzer());
         String rules =
             "&,and\n" +
             "come,advance,approach\n";
@@ -50,7 +50,7 @@ public void testLenientParser() throws IOException, ParseException {
     }
 
     public void testNonLenientParser() {
-        ElasticSynonymParser parser = new ElasticSynonymParser(true, false, false, new StandardAnalyzer());
+        ElasticsearchSynonymParser parser = new ElasticsearchSynonymParser(true, false, false, new StandardAnalyzer());
         String rules =
             "&,and=>and\n" +
             "come,advance,approach\n";