From 30006cff7e098c7bd5b36952668a804aac03821b Mon Sep 17 00:00:00 2001
From: Michael McCandless <mikemccand@apache.org>
Date: Thu, 1 May 2014 16:28:21 +0000
Subject: [PATCH] LUCENE-5611: don't abort segment if term vector settings are
 wrong

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1591700 13f79535-47bb-0310-9956-ffa450edef68
---
 .../lucene/index/DefaultIndexingChain.java    | 44 +++++++------------
 .../lucene/index/TestTermVectorsWriter.java   | 30 +++++++++++++
 2 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java b/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java
index e0a411b0f67..646b71dc8b6 100644
--- a/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java
+++ b/lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java
@@ -321,49 +321,37 @@ final class DefaultIndexingChain extends DocConsumer {
     }
 
     // Add stored fields:
-    // TODO: if these hit exc today ->>> corrumption!
     fillStoredFields(docState.docID);
     startStoredFields();
 
-    // TODO: clean up this loop, it's complicated because dv exceptions are non-aborting,
-    // but storedfields are. Its also bogus that docvalues are treated as stored fields...
-    for (StorableField field : docState.doc.storableFields()) {
-      final String fieldName = field.name();
-      IndexableFieldType fieldType = field.fieldType();
-      PerField fp = null;
+    // TODO: clean up this loop, it's bogus that docvalues are treated as stored fields...
+    boolean abort = false;
+    try {
+      for (StorableField field : docState.doc.storableFields()) {
+        String fieldName = field.name();
+        IndexableFieldType fieldType = field.fieldType();
       
-      success = false;
-      try {
-        // TODO: make this non-aborting and change the test to confirm that!!!
         verifyFieldType(fieldName, fieldType);
         
-        fp = getOrAddField(fieldName, fieldType, false);
+        PerField fp = getOrAddField(fieldName, fieldType, false);
         if (fieldType.stored()) {
+          abort = true;
           storedFieldsWriter.writeField(fp.fieldInfo, field);
+          abort = false;
         }
-        success = true;
-      } finally {
-        if (!success) {
-          docWriter.setAborting();
-        }
-      }
-      
-      success = false;
-      try {
+
         DocValuesType dvType = fieldType.docValueType();
         if (dvType != null) {
           indexDocValue(fp, dvType, field);
         }
-        success = true;
-      } finally {
-        if (!success) {
-          // dv failed: so just try to bail on the current doc by calling finishDocument()...
-          finishStoredFields();
-        }
+      }
+    } finally {
+      if (abort) {
+        docWriter.setAborting();
+      } else {
+        finishStoredFields();
       }
     }
-
-    finishStoredFields();
   }
 
   private static void verifyFieldType(String name, IndexableFieldType ft) {
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestTermVectorsWriter.java b/lucene/core/src/test/org/apache/lucene/index/TestTermVectorsWriter.java
index 690ebe9004d..4de4c908be4 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestTermVectorsWriter.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestTermVectorsWriter.java
@@ -18,6 +18,7 @@ package org.apache.lucene.index;
  */
 
 import java.io.IOException;
+
 import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.CachingTokenFilter;
 import org.apache.lucene.analysis.MockAnalyzer;
@@ -27,6 +28,7 @@ import org.apache.lucene.analysis.TokenStream;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.FieldType;
+import org.apache.lucene.document.StoredField;
 import org.apache.lucene.document.StringField;
 import org.apache.lucene.document.TextField;
 import org.apache.lucene.search.DocIdSetIterator;
@@ -660,4 +662,32 @@ public class TestTermVectorsWriter extends LuceneTestCase {
     iw.close();
     dir.close();
   }
+
+  // LUCENE-5611: don't abort segment when term vector settings are wrong
+  public void testNoAbortOnBadTVSettings() throws Exception {
+    Directory dir = newDirectory();
+    // Don't use RandomIndexWriter because we want to be sure both docs go to 1 seg:
+    IndexWriterConfig iwc = new IndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random()));
+    IndexWriter iw = new IndexWriter(dir, iwc);
+
+    Document doc = new Document();
+    iw.addDocument(doc);
+    FieldType ft = new FieldType(StoredField.TYPE);
+    ft.setStoreTermVectors(true);
+    ft.freeze();
+    doc.add(new Field("field", "value", ft));
+    try {
+      iw.addDocument(doc);
+      fail("should have hit exc");
+    } catch (IllegalArgumentException iae) {
+      // expected
+    }
+    IndexReader r = DirectoryReader.open(iw, true);
+
+    // Make sure the exc didn't lose our first document:
+    assertEquals(1, r.numDocs());
+    iw.close();
+    r.close();
+    dir.close();
+  }
 }