You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by si...@apache.org on 2011/03/25 09:48:03 UTC
svn commit: r1085288 - in /lucene/dev/trunk/lucene/src:
java/org/apache/lucene/index/FieldInfos.java
test/org/apache/lucene/index/TestFieldInfos.java
Author: simonw
Date: Fri Mar 25 08:48:02 2011
New Revision: 1085288
URL: http://svn.apache.org/viewvc?rev=1085288&view=rev
Log:
LUCENE-2983: FieldInfos should be read-only if loaded from disk
Modified:
lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/FieldInfos.java
lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestFieldInfos.java
Modified: lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/FieldInfos.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/FieldInfos.java?rev=1085288&r1=1085287&r2=1085288&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/FieldInfos.java (original)
+++ lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/FieldInfos.java Fri Mar 25 08:48:02 2011
@@ -213,34 +213,49 @@ public final class FieldInfos implements
private int format;
+ /**
+ * Creates a new {@link FieldInfos} instance with a private
+ * {@link FieldNumberBiMap}.
+ * <p>
+ * Note: this ctor should not be used during indexing use
+ * {@link FieldInfos#FieldInfos(FieldInfos)} or
+ * {@link FieldInfos#FieldInfos(FieldNumberBiMap)} instead.
+ */
public FieldInfos() {
this(new FieldNumberBiMap());
}
+ /**
+ * Creates a new {@link FieldInfo} instance from the given instance. If the given instance is
+ * read-only this instance will be read-only too.
+ *
+ * @see #isReadOnly()
+ */
FieldInfos(FieldInfos other) {
this(other.globalFieldNumbers);
}
-
+
+ /**
+ * Creates a new FieldInfos instance with the given {@link FieldNumberBiMap}.
+ * If the {@link FieldNumberBiMap} is <code>null</code> this instance will be read-only.
+ * @see #isReadOnly()
+ */
FieldInfos(FieldNumberBiMap globalFieldNumbers) {
this.globalFieldNumbers = globalFieldNumbers;
}
/**
* Construct a FieldInfos object using the directory and the name of the file
- * IndexInput
+ * IndexInput.
+ * <p>
+ * Note: The created instance will be read-only
+ *
* @param d The directory to open the IndexInput from
* @param name The name of the file to open the IndexInput from in the Directory
* @throws IOException
*/
public FieldInfos(Directory d, String name) throws IOException {
- this(new FieldNumberBiMap());
- /*
- * TODO: in the read case we create a FNBM for each FIs which is a wast of resources.
- * Yet, we must not seed this with a global map since due to addIndexes(Dir) we could
- * have non-matching field numbers. we should use a null FNBM here and set the FIs
- * to READ-ONLY once this ctor is done. Each modificator should check if we are readonly
- * with an assert
- */
+ this((FieldNumberBiMap)null); // use null here to make this FIs Read-Only
IndexInput input = d.openInput(name);
try {
read(input, name);
@@ -257,7 +272,7 @@ public final class FieldInfos implements
private void putInternal(FieldInfo fi) {
assert !byNumber.containsKey(fi.number);
assert !byName.containsKey(fi.name);
- assert globalFieldNumbers.containsConsistent(Integer.valueOf(fi.number), fi.name);
+ assert globalFieldNumbers == null || globalFieldNumbers.containsConsistent(Integer.valueOf(fi.number), fi.name);
byNumber.put(fi.number, fi);
byName.put(fi.name, fi);
}
@@ -404,10 +419,12 @@ public final class FieldInfos implements
synchronized private FieldInfo addOrUpdateInternal(String name, int preferredFieldNumber, boolean isIndexed,
boolean storeTermVector, boolean storePositionWithTermVector, boolean storeOffsetWithTermVector,
boolean omitNorms, boolean storePayloads, boolean omitTermFreqAndPositions) {
-
- FieldInfo fi = fieldInfo(name);
+ if (globalFieldNumbers == null) {
+ throw new IllegalStateException("FieldInfos are read-only, create a new instance with a global field map to make modifications to FieldInfos");
+ }
+ final FieldInfo fi = fieldInfo(name);
if (fi == null) {
- int fieldNumber = nextFieldNumber(name, preferredFieldNumber);
+ final int fieldNumber = nextFieldNumber(name, preferredFieldNumber);
return addInternal(name, fieldNumber, isIndexed, storeTermVector, storePositionWithTermVector, storeOffsetWithTermVector, omitNorms, storePayloads, omitTermFreqAndPositions);
} else {
fi.update(isIndexed, storeTermVector, storePositionWithTermVector, storeOffsetWithTermVector, omitNorms, storePayloads, omitTermFreqAndPositions);
@@ -422,16 +439,21 @@ public final class FieldInfos implements
fi.omitNorms, fi.storePayloads,
fi.omitTermFreqAndPositions);
}
-
+
+ /*
+ * NOTE: if you call this method from a public method make sure you check if we are modifiable and throw an exception otherwise
+ */
private FieldInfo addInternal(String name, int fieldNumber, boolean isIndexed,
boolean storeTermVector, boolean storePositionWithTermVector,
boolean storeOffsetWithTermVector, boolean omitNorms, boolean storePayloads, boolean omitTermFreqAndPositions) {
+ // don't check modifiable here since we use that to initially build up FIs
name = StringHelper.intern(name);
- globalFieldNumbers.setIfNotSet(fieldNumber, name);
- FieldInfo fi = new FieldInfo(name, isIndexed, fieldNumber, storeTermVector, storePositionWithTermVector,
+ if (globalFieldNumbers != null) {
+ globalFieldNumbers.setIfNotSet(fieldNumber, name);
+ }
+ final FieldInfo fi = new FieldInfo(name, isIndexed, fieldNumber, storeTermVector, storePositionWithTermVector,
storeOffsetWithTermVector, omitNorms, storePayloads, omitTermFreqAndPositions);
- assert byNumber.get(fi.number) == null;
putInternal(fi);
return fi;
}
@@ -453,8 +475,8 @@ public final class FieldInfos implements
* with the given number doesn't exist.
*/
public String fieldName(int fieldNumber) {
- FieldInfo fi = fieldInfo(fieldNumber);
- return (fi != null) ? fi.name : "";
+ FieldInfo fi = fieldInfo(fieldNumber);
+ return (fi != null) ? fi.name : "";
}
/**
@@ -502,6 +524,16 @@ public final class FieldInfos implements
output.close();
}
}
+
+ /**
+ * Returns <code>true</code> iff this instance is not backed by a
+ * {@link FieldNumberBiMap}. Instances read from a directory via
+ * {@link FieldInfos#FieldInfos(Directory, String)} will always be read-only
+ * since no {@link FieldNumberBiMap} is supplied, otherwise <code>false</code>.
+ */
+ public final boolean isReadOnly() {
+ return globalFieldNumbers == null;
+ }
public void write(IndexOutput output) throws IOException {
output.writeVInt(FORMAT_CURRENT);
Modified: lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestFieldInfos.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestFieldInfos.java?rev=1085288&r1=1085287&r2=1085288&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestFieldInfos.java (original)
+++ lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestFieldInfos.java Fri Mar 25 08:48:02 2011
@@ -24,6 +24,7 @@ import org.apache.lucene.store.Directory
import org.apache.lucene.store.IndexOutput;
import java.io.IOException;
+import java.util.Arrays;
//import org.cnlp.utils.properties.ResourceBundleHelper;
@@ -37,44 +38,124 @@ public class TestFieldInfos extends Luce
DocHelper.setupDoc(testDoc);
}
- public void test() throws IOException {
- //Positive test of FieldInfos
+ public FieldInfos createAndWriteFieldInfos(Directory dir, String filename) throws IOException{
+ //Positive test of FieldInfos
assertTrue(testDoc != null);
FieldInfos fieldInfos = new FieldInfos();
_TestUtil.add(testDoc, fieldInfos);
//Since the complement is stored as well in the fields map
assertTrue(fieldInfos.size() == DocHelper.all.size()); //this is all b/c we are using the no-arg constructor
- Directory dir = newDirectory();
- String name = "testFile";
- IndexOutput output = dir.createOutput(name);
+
+
+ IndexOutput output = dir.createOutput(filename);
assertTrue(output != null);
//Use a RAMOutputStream
-
- fieldInfos.write(output);
- output.close();
- assertTrue(dir.fileLength(name) > 0);
- FieldInfos readIn = new FieldInfos(dir, name);
- assertTrue(fieldInfos.size() == readIn.size());
- FieldInfo info = readIn.fieldInfo("textField1");
- assertTrue(info != null);
- assertTrue(info.storeTermVector == false);
- assertTrue(info.omitNorms == false);
-
- info = readIn.fieldInfo("textField2");
- assertTrue(info != null);
- assertTrue(info.storeTermVector == true);
- assertTrue(info.omitNorms == false);
-
- info = readIn.fieldInfo("textField3");
- assertTrue(info != null);
- assertTrue(info.storeTermVector == false);
- assertTrue(info.omitNorms == true);
-
- info = readIn.fieldInfo("omitNorms");
- assertTrue(info != null);
- assertTrue(info.storeTermVector == false);
- assertTrue(info.omitNorms == true);
+
+ fieldInfos.write(output);
+ output.close();
+ return fieldInfos;
+ }
+ public void test() throws IOException {
+ String name = "testFile";
+ Directory dir = newDirectory();
+ FieldInfos fieldInfos = createAndWriteFieldInfos(dir, name);
+ assertTrue(dir.fileLength(name) > 0);
+ FieldInfos readIn = new FieldInfos(dir, name);
+ assertTrue(fieldInfos.size() == readIn.size());
+ FieldInfo info = readIn.fieldInfo("textField1");
+ assertTrue(info != null);
+ assertTrue(info.storeTermVector == false);
+ assertTrue(info.omitNorms == false);
+
+ info = readIn.fieldInfo("textField2");
+ assertTrue(info != null);
+ assertTrue(info.storeTermVector == true);
+ assertTrue(info.omitNorms == false);
+
+ info = readIn.fieldInfo("textField3");
+ assertTrue(info != null);
+ assertTrue(info.storeTermVector == false);
+ assertTrue(info.omitNorms == true);
+
+ info = readIn.fieldInfo("omitNorms");
+ assertTrue(info != null);
+ assertTrue(info.storeTermVector == false);
+ assertTrue(info.omitNorms == true);
- dir.close();
+ dir.close();
+ }
+
+ public void testReadOnly() throws IOException {
+ String name = "testFile";
+ Directory dir = newDirectory();
+ FieldInfos fieldInfos = createAndWriteFieldInfos(dir, name);
+ FieldInfos readOnly = new FieldInfos(dir, name);
+ assertReadOnly(readOnly, fieldInfos);
+ FieldInfos readOnlyClone = (FieldInfos)readOnly.clone();
+ assertNotSame(readOnly, readOnlyClone);
+ // clone is also read only - no global field map
+ assertReadOnly(readOnlyClone, fieldInfos);
+ dir.close();
+ }
+
+ private void assertReadOnly(FieldInfos readOnly, FieldInfos modifiable) {
+ assertTrue(readOnly.isReadOnly());
+ assertFalse(modifiable.isReadOnly());
+ try {
+ readOnly.add(modifiable.fieldInfo(0));
+ fail("instance should be read only");
+ } catch (IllegalStateException e) {
+ // expected
+ }
+
+ try {
+ readOnly.add("bogus", random.nextBoolean());
+ fail("instance should be read only");
+ } catch (IllegalStateException e) {
+ // expected
+ }
+ try {
+ readOnly.add("bogus", random.nextBoolean(), random.nextBoolean());
+ fail("instance should be read only");
+ } catch (IllegalStateException e) {
+ // expected
+ }
+ try {
+ readOnly.add("bogus", random.nextBoolean(), random.nextBoolean(),
+ random.nextBoolean(), random.nextBoolean());
+ fail("instance should be read only");
+ } catch (IllegalStateException e) {
+ // expected
+ }
+ try {
+ readOnly.add("bogus", random.nextBoolean(), random.nextBoolean(),
+ random.nextBoolean(), random.nextBoolean(), random.nextBoolean());
+ fail("instance should be read only");
+ } catch (IllegalStateException e) {
+ // expected
+ }
+ try {
+ readOnly.add("bogus", random.nextBoolean(), random.nextBoolean(),
+ random.nextBoolean(), random.nextBoolean(), random.nextBoolean(),
+ random.nextBoolean(), random.nextBoolean());
+ fail("instance should be read only");
+ } catch (IllegalStateException e) {
+ // expected
+ }
+ try {
+ readOnly.add(Arrays.asList("a", "b", "c"), random.nextBoolean());
+ fail("instance should be read only");
+ } catch (IllegalStateException e) {
+ // expected
+ }
+
+ assertEquals(modifiable.size(), readOnly.size());
+ // assert we can iterate
+ for (FieldInfo fi : readOnly) {
+ assertEquals(fi.name, modifiable.fieldName(fi.number));
+ }
+
}
+
+
}