Index: lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java =================================================================== --- lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java (revision 1244909) +++ lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java (working copy) @@ -1,19 +1,21 @@ package org.apache.lucene.facet.taxonomy.directory; +import java.io.IOException; import java.util.HashMap; import java.util.Map; +import org.apache.lucene.facet.taxonomy.CategoryPath; +import org.apache.lucene.facet.taxonomy.InconsistentTaxonomyException; +import org.apache.lucene.facet.taxonomy.writercache.TaxonomyWriterCache; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; import org.apache.lucene.index.IndexWriterConfig.OpenMode; import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.Directory; +import org.apache.lucene.util.LuceneTestCase; import org.junit.Test; -import org.apache.lucene.util.LuceneTestCase; -import org.apache.lucene.facet.taxonomy.CategoryPath; -import org.apache.lucene.facet.taxonomy.directory.DirectoryTaxonomyWriter; -import org.apache.lucene.facet.taxonomy.writercache.TaxonomyWriterCache; - /** * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with @@ -67,23 +69,38 @@ @Test public void testCommitUserData() throws Exception { - // Verifies that committed data is retrievable + // Verifies taxonomy commit data Directory dir = newDirectory(); - DirectoryTaxonomyWriter ltw = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache()); - assertFalse(IndexReader.indexExists(dir)); - ltw.commit(); // first commit, so that an index will be created - ltw.addCategory(new CategoryPath("a")); - ltw.addCategory(new CategoryPath("b")); + DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache()); + taxoWriter.addCategory(new CategoryPath("a")); + taxoWriter.addCategory(new CategoryPath("b")); Map userCommitData = new HashMap(); userCommitData.put("testing", "1 2 3"); - ltw.commit(userCommitData); - ltw.close(); + taxoWriter.commit(userCommitData); + taxoWriter.close(); IndexReader r = IndexReader.open(dir); assertEquals("2 categories plus root should have been committed to the underlying directory", 3, r.numDocs()); Map readUserCommitData = r.getIndexCommit().getUserData(); assertTrue("wrong value extracted from commit data", "1 2 3".equals(readUserCommitData.get("testing"))); + assertNotNull("index.create.time not found in commitData", readUserCommitData.get(DirectoryTaxonomyWriter.INDEX_CREATE_TIME)); r.close(); + + // open DirTaxoWriter again and commit, INDEX_CREATE_TIME should still exist + // in the commit data, otherwise DirTaxoReader.refresh() might not detect + // that the taxonomy index has been recreated. + taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache()); + taxoWriter.addCategory(new CategoryPath("c")); // add a category so that commit will happen + taxoWriter.commit(new HashMap(){{ + put("just", "data"); + }}); + taxoWriter.close(); + + r = IndexReader.open(dir); + readUserCommitData = r.getIndexCommit().getUserData(); + assertNotNull("index.create.time not found in commitData", readUserCommitData.get(DirectoryTaxonomyWriter.INDEX_CREATE_TIME)); + r.close(); + dir.close(); } @@ -117,5 +134,71 @@ } dir.close(); } + + private void touchTaxo(DirectoryTaxonomyWriter taxoWriter, CategoryPath cp) throws IOException { + taxoWriter.addCategory(cp); + taxoWriter.commit(new HashMap(){{ + put("just", "data"); + }}); + } + @Test + public void testRecreateAndRefresh() throws Exception { + // DirTaxoWriter lost the INDEX_CREATE_TIME property if it was opened in + // CREATE_OR_APPEND (or commit(userData) called twice), which could lead to + // DirTaxoReader succeeding to refresh(). + Directory dir = newDirectory(); + + DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache()); + touchTaxo(taxoWriter, new CategoryPath("a")); + + DirectoryTaxonomyReader taxoReader = new DirectoryTaxonomyReader(dir); + + touchTaxo(taxoWriter, new CategoryPath("b")); + + // this should not fail + taxoReader.refresh(); + + // now recreate the taxonomy, and check that the timestamp is preserved after opening DirTW again. + taxoWriter.close(); + taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE, new NoOpCache()); + touchTaxo(taxoWriter, new CategoryPath("c")); + taxoWriter.close(); + + taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache()); + touchTaxo(taxoWriter, new CategoryPath("d")); + taxoWriter.close(); + + // this should fail + try { + taxoReader.refresh(); + fail("IconsistentTaxonomyException should have been thrown"); + } catch (InconsistentTaxonomyException e) { + // ok, expected + } + + taxoReader.close(); + dir.close(); + } + + @Test + public void testUndefinedCreateTime() throws Exception { + // tests that if the taxonomy index doesn't have the INDEX_CREATE_TIME + // property (supports pre-3.6 indexes), all still works. + Directory dir = newDirectory(); + + // create an empty index first, so that DirTaxoWriter initializes createTime to null. + new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, null)).close(); + + DirectoryTaxonomyWriter taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, new NoOpCache()); + // we cannot commit null keys/values, this ensures that if DirTW.createTime is null, we can still commit. + taxoWriter.close(); + + DirectoryTaxonomyReader taxoReader = new DirectoryTaxonomyReader(dir); + taxoReader.refresh(); + taxoReader.close(); + + dir.close(); + } + } Index: lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyReader.java =================================================================== --- lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyReader.java (revision 1244909) +++ lucene/contrib/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyReader.java (working copy) @@ -151,7 +151,7 @@ tw.close(); tr = new DirectoryTaxonomyReader(dir); - int baseNumcategories = tr.getSize(); + int baseNumCategories = tr.getSize(); for (int i=0; i - * Applications making use of {@link TaxonomyWriter#commit(Map)} should not use this - * particular property name. + * Applications should not use this property in their commit data because it + * will be overridden by this taxonomy writer. */ public static final String INDEX_CREATE_TIME = "index.create.time"; - + private IndexWriter indexWriter; private int nextID; private char delimiter = Consts.DEFAULT_DELIMITER; @@ -111,11 +112,8 @@ private IndexReader reader; private int cacheMisses; - /** - * When a taxonomy is created, we mark that its create time should be committed in the - * next commit. - */ - private String taxoIndexCreateTime = null; + /** Records the taxonomy index creation time. */ + private final String createTime; /** * setDelimiter changes the character that the taxonomy uses in its internal @@ -180,12 +178,20 @@ * if another error occurred. */ public DirectoryTaxonomyWriter(Directory directory, OpenMode openMode, - TaxonomyWriterCache cache) - throws CorruptIndexException, LockObtainFailedException, - IOException { + TaxonomyWriterCache cache) throws IOException { - if (!IndexReader.indexExists(directory) || openMode==OpenMode.CREATE) { - taxoIndexCreateTime = Long.toString(System.nanoTime()); + if (!IndexReader.indexExists(directory) || openMode == OpenMode.CREATE) { + createTime = Long.toString(System.nanoTime()); + } else { + Map commitData = IndexReader.getCommitUserData(directory); + if (commitData != null) { + // It is ok if an existing index doesn't have commitData, or the + // INDEX_CREATE_TIME property. If ever it will be recreated, we'll set + // createTime accordingly in the above 'if'. + createTime = commitData.get(INDEX_CREATE_TIME); + } else { + createTime = null; + } } IndexWriterConfig config = createIndexWriterConfig(openMode); @@ -204,7 +210,7 @@ this.nextID = indexWriter.maxDoc(); - if (cache==null) { + if (cache == null) { cache = defaultTaxonomyWriterCache(); } this.cache = cache; @@ -320,10 +326,7 @@ */ public synchronized void close() throws CorruptIndexException, IOException { if (indexWriter != null) { - if (taxoIndexCreateTime != null) { - indexWriter.commit(combinedCommitData(null)); - taxoIndexCreateTime = null; - } + indexWriter.commit(combinedCommitData(null)); doClose(); } } @@ -626,12 +629,7 @@ */ public synchronized void commit() throws CorruptIndexException, IOException { ensureOpen(); - if (taxoIndexCreateTime != null) { - indexWriter.commit(combinedCommitData(null)); - taxoIndexCreateTime = null; - } else { - indexWriter.commit(); - } + indexWriter.commit(combinedCommitData(null)); refreshReader(); } @@ -643,7 +641,9 @@ if (userData != null) { m.putAll(userData); } - m.put(INDEX_CREATE_TIME, taxoIndexCreateTime); + if (createTime != null) { + m.put(INDEX_CREATE_TIME, createTime); + } return m; } @@ -654,12 +654,7 @@ */ public synchronized void commit(Map commitUserData) throws CorruptIndexException, IOException { ensureOpen(); - if (taxoIndexCreateTime != null) { - indexWriter.commit(combinedCommitData(commitUserData)); - taxoIndexCreateTime = null; - } else { - indexWriter.commit(commitUserData); - } + indexWriter.commit(combinedCommitData(commitUserData)); refreshReader(); } @@ -669,12 +664,7 @@ */ public synchronized void prepareCommit() throws CorruptIndexException, IOException { ensureOpen(); - if (taxoIndexCreateTime != null) { - indexWriter.prepareCommit(combinedCommitData(null)); - taxoIndexCreateTime = null; - } else { - indexWriter.prepareCommit(); - } + indexWriter.prepareCommit(combinedCommitData(null)); } /** @@ -683,12 +673,7 @@ */ public synchronized void prepareCommit(Map commitUserData) throws CorruptIndexException, IOException { ensureOpen(); - if (taxoIndexCreateTime != null) { - indexWriter.prepareCommit(combinedCommitData(commitUserData)); - taxoIndexCreateTime = null; - } else { - indexWriter.prepareCommit(commitUserData); - } + indexWriter.prepareCommit(combinedCommitData(commitUserData)); } /** Index: lucene/contrib/CHANGES.txt =================================================================== --- lucene/contrib/CHANGES.txt (revision 1244909) +++ lucene/contrib/CHANGES.txt (working copy) @@ -140,7 +140,12 @@ * SOLR-3076: ToParent/ChildBlockJoinQuery was not handling deleted docs correctly (Mikhail Khludnev via Mike McCandless). - + + * LUCENE-3794: DirectoryTaxonomyWriter could lose the INDEX_CREATE_TIME + property if multiple commits with userData were done. It now always records + the creation time in the taxonomy index commitData, and reads it from the + index in the constructor. (Shai Erera) + Documentation * LUCENE-3599: Javadocs for DistanceUtils.haversine() were incorrectly