NightOwl888 commented on code in PR #948:
URL: https://github.com/apache/lucenenet/pull/948#discussion_r1827590212
##########
src/Lucene.Net.Tests/Index/TestIndexWriter.cs:
##########
@@ -2575,6 +2589,49 @@ public virtual void TestGetCommitData()
dir.Dispose();
}
+ // LUCENENET-specific: backport fix and test from Lucene 9.9.0
(lucene#12626, lucene#12637)
+ [Test]
+ public void TestGetCommitDataFromOldSnapshot()
+ {
+ Directory dir = NewDirectory();
+ IndexWriter writer = new IndexWriter(dir,
NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(Random)));
+ // LUCENENET TODO: in a post-4.8 port, this should use
SetLiveCommitData
+ writer.SetCommitData(new Dictionary<string, string>
+ {
+ { "key", "value" },
+ });
+ assertEquals("value", GetLiveCommitData(writer)["key"]);
+ writer.Commit();
+ // Snapshot this commit to open later
+ IndexCommit indexCommit =
+
((SnapshotDeletionPolicy)writer.Config.IndexDeletionPolicy).Snapshot();
+ writer.Dispose();
+
+ // Modify the commit data and commit on close so the most recent
commit data is different
+ writer = new IndexWriter(dir,
NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(Random)));
Review Comment:
In the upstream code the analyzer was `null` rather than `new
MockAnalyzer(Random)`. Looking at the `LiveIndexWriterConfig`, it should accept
that, although I am not sure about the code that uses `IndexWriterConfig`. Did
you try `null`, and if so, what happened?
If it doesn't work, then maybe we should patch this inside of
`NewSnapshotIndexWriterConfig` for the time being, since that will come up as a
new method that we need to port, anyway.
Either way, this diverges from upstream and we will need a `LUCENENET
UPGRADE TODO` comment (unless `null` works).
##########
src/Lucene.Net.Tests/Index/TestIndexWriter.cs:
##########
@@ -2556,12 +2556,26 @@ public virtual void TestCommitWithUserDataOnly()
dir.Dispose();
}
+ // LUCENENET-specific: backport fix and test from Lucene 9.9.0
(lucene#12626, lucene#12637)
+ private Dictionary<string, string> GetLiveCommitData(IndexWriter
writer)
+ {
+ Dictionary<string, string> data = new Dictionary<string, string>();
+ // LUCENENET TODO: in a post-4.8 port, this should use
LiveCommitData
Review Comment:
`LUCENENET TODO` is generally reserved for work to do *prior to the 4.8.0
release* (except Ron also committed a bunch of them that will apply to an
upgrade just like these). We should have a different convention for future work
than for things that are pending for the current release. We specifically
mention in the CONTRIBUTING section that `LUCENENET TODO` are up for grabs, but
we definitely don't want this work done until later.
Maybe we should make it `LUCENENET UPGRADE TODO` to ensure it doesn't come
up in a search for `LUCENENET TODO` (and change all of those that Ron added as
well in a separate PR). It would then be easy to do a find and replace before
we begin upgrading to change them to `LUCENENET TODO`.
##########
src/Lucene.Net.Tests/Index/TestIndexWriter.cs:
##########
@@ -2575,6 +2589,49 @@ public virtual void TestGetCommitData()
dir.Dispose();
}
+ // LUCENENET-specific: backport fix and test from Lucene 9.9.0
(lucene#12626, lucene#12637)
+ [Test]
+ public void TestGetCommitDataFromOldSnapshot()
+ {
+ Directory dir = NewDirectory();
+ IndexWriter writer = new IndexWriter(dir,
NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(Random)));
+ // LUCENENET TODO: in a post-4.8 port, this should use
SetLiveCommitData
Review Comment:
See my other comment about why `LUCENENET TODO` is not appropriate here.
##########
src/Lucene.Net.Tests/Index/TestIndexWriter.cs:
##########
@@ -2575,6 +2589,49 @@ public virtual void TestGetCommitData()
dir.Dispose();
}
+ // LUCENENET-specific: backport fix and test from Lucene 9.9.0
(lucene#12626, lucene#12637)
+ [Test]
+ public void TestGetCommitDataFromOldSnapshot()
+ {
+ Directory dir = NewDirectory();
+ IndexWriter writer = new IndexWriter(dir,
NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(Random)));
+ // LUCENENET TODO: in a post-4.8 port, this should use
SetLiveCommitData
+ writer.SetCommitData(new Dictionary<string, string>
+ {
+ { "key", "value" },
+ });
+ assertEquals("value", GetLiveCommitData(writer)["key"]);
+ writer.Commit();
+ // Snapshot this commit to open later
+ IndexCommit indexCommit =
+
((SnapshotDeletionPolicy)writer.Config.IndexDeletionPolicy).Snapshot();
+ writer.Dispose();
+
+ // Modify the commit data and commit on close so the most recent
commit data is different
+ writer = new IndexWriter(dir,
NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(Random)));
+ // LUCENENET TODO: in a post-4.8 port, this should use
SetLiveCommitData
Review Comment:
See my other comment about why `LUCENENET TODO` is not appropriate here.
##########
src/Lucene.Net.Tests/Index/TestIndexWriter.cs:
##########
@@ -2575,6 +2589,49 @@ public virtual void TestGetCommitData()
dir.Dispose();
}
+ // LUCENENET-specific: backport fix and test from Lucene 9.9.0
(lucene#12626, lucene#12637)
+ [Test]
+ public void TestGetCommitDataFromOldSnapshot()
+ {
+ Directory dir = NewDirectory();
+ IndexWriter writer = new IndexWriter(dir,
NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(Random)));
+ // LUCENENET TODO: in a post-4.8 port, this should use
SetLiveCommitData
+ writer.SetCommitData(new Dictionary<string, string>
+ {
+ { "key", "value" },
+ });
+ assertEquals("value", GetLiveCommitData(writer)["key"]);
+ writer.Commit();
+ // Snapshot this commit to open later
+ IndexCommit indexCommit =
+
((SnapshotDeletionPolicy)writer.Config.IndexDeletionPolicy).Snapshot();
+ writer.Dispose();
+
+ // Modify the commit data and commit on close so the most recent
commit data is different
+ writer = new IndexWriter(dir,
NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(Random)));
+ // LUCENENET TODO: in a post-4.8 port, this should use
SetLiveCommitData
+ writer.SetCommitData(new Dictionary<string, string>()
+ {
+ { "key", "value2" },
+ });
+
+ assertEquals("value2", GetLiveCommitData(writer)["key"]);
+ writer.Dispose();
+
+ // validate that when opening writer from older snapshotted index
commit, the old commit data is
+ // visible
+ writer =
+ new IndexWriter(
+ dir,
+ NewSnapshotIndexWriterConfig(TEST_VERSION_CURRENT, new
MockAnalyzer(Random))
Review Comment:
See my other comment about `null` vs `new MockAnalyzer(Random)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
