diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java index b705f4a3c..7bc56482e 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/DocumentIndexWriter.java @@ -20,11 +20,14 @@ import org.dizitart.no2.collection.Document; import org.dizitart.no2.common.FieldValues; import org.dizitart.no2.common.Fields; +import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.common.util.DocumentUtils; import org.dizitart.no2.index.IndexDescriptor; import org.dizitart.no2.index.NitriteIndexer; import java.util.Collection; +import java.util.Objects; +import java.util.List; /** * @since 4.0 @@ -73,6 +76,15 @@ void updateIndexEntry(Document oldDocument, Document newDocument, Document updat // if the index is affected by the update if (DocumentUtils.isAffectedByUpdate(fields, updatedFields)) { + // "affected" only means the update carries the field. An update that + // writes the whole document back, the common upsert shape, carries every + // indexed field with its old value, and rewriting those entries is pure + // cost. A dirty index still has to be rebuilt, so that case is not skipped. + if (!indexOperations.shouldRebuildIndex(fields) + && sameIndexedValues(oldDocument, newDocument, fields)) { + continue; + } + String indexType = indexDescriptor.getIndexType(); NitriteIndexer nitriteIndexer = nitriteConfig.findIndexer(indexType); @@ -83,6 +95,25 @@ void updateIndexEntry(Document oldDocument, Document newDocument, Document updat } } + /** + * Whether the two documents hold the same values for every field of the index, compared + * deeply so that arrays and embedded values count as equal when their contents are. + */ + private static boolean sameIndexedValues(Document oldDocument, Document newDocument, Fields fields) { + List> before = DocumentUtils.getValues(oldDocument, fields).getValues(); + List> after = DocumentUtils.getValues(newDocument, fields).getValues(); + if (before.size() != after.size()) { + return false; + } + for (int i = 0; i < before.size(); i++) { + if (!Objects.equals(before.get(i).getFirst(), after.get(i).getFirst()) + || !Objects.deepEquals(before.get(i).getSecond(), after.get(i).getSecond())) { + return false; + } + } + return true; + } + private void writeIndexEntryInternal(IndexDescriptor indexDescriptor, Document document, NitriteIndexer nitriteIndexer) { if (indexDescriptor != null) { diff --git a/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java b/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java index 42edb9745..bcf4c292e 100644 --- a/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/collection/operation/DocumentIndexWriterTest.java @@ -21,6 +21,7 @@ import org.dizitart.no2.common.Fields; import org.dizitart.no2.index.IndexDescriptor; import org.dizitart.no2.index.IndexType; +import org.dizitart.no2.index.NitriteIndexer; import org.dizitart.no2.index.UniqueIndexer; import org.dizitart.no2.store.memory.InMemoryStore; import org.junit.Test; @@ -28,6 +29,7 @@ import java.util.ArrayList; import static org.dizitart.no2.collection.Document.createDocument; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; public class DocumentIndexWriterTest { @@ -72,5 +74,67 @@ public void testUpdateIndexEntry() { (new DocumentIndexWriter(nitriteConfig, indexOperations)).updateIndexEntry(createDocument("a", 1), createDocument("a", 2), createDocument("a", 3)); verify(indexOperations).listIndexes(); } -} + @Test + public void testUpdateSkipsIndexWhenIndexedValueIsUnchanged() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + DocumentIndexWriter writer = writerWithIndexOn("a", indexer, false); + + writer.updateIndexEntry(createDocument("a", 1).put("b", "old"), + createDocument("a", 1).put("b", "new"), + createDocument("a", 1).put("b", "new")); + + verify(indexer, never()).removeIndexEntry(any(), any(), any()); + verify(indexer, never()).writeIndexEntry(any(), any(), any()); + } + + @Test + public void testUpdateSkipsIndexWhenArrayValueHasSameContents() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + DocumentIndexWriter writer = writerWithIndexOn("a", indexer, false); + + writer.updateIndexEntry(createDocument("a", new int[]{1, 2}), + createDocument("a", new int[]{1, 2}), + createDocument("a", new int[]{1, 2})); + + verify(indexer, never()).removeIndexEntry(any(), any(), any()); + verify(indexer, never()).writeIndexEntry(any(), any(), any()); + } + + @Test + public void testUpdateRewritesIndexWhenIndexedValueChanges() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + DocumentIndexWriter writer = writerWithIndexOn("a", indexer, false); + + writer.updateIndexEntry(createDocument("a", 1), createDocument("a", 2), createDocument("a", 2)); + + verify(indexer).removeIndexEntry(any(), any(), any()); + verify(indexer).writeIndexEntry(any(), any(), any()); + } + + @Test + public void testUpdateStillRebuildsDirtyIndexWhenValueIsUnchanged() { + NitriteIndexer indexer = mock(NitriteIndexer.class); + IndexOperations indexOperations = mock(IndexOperations.class); + IndexDescriptor descriptor = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames("a"), "c"); + when(indexOperations.listIndexes()).thenReturn(new ArrayList<>(java.util.List.of(descriptor))); + when(indexOperations.shouldRebuildIndex(any())).thenReturn(true); + NitriteConfig nitriteConfig = mock(NitriteConfig.class); + doReturn(indexer).when(nitriteConfig).findIndexer(IndexType.NON_UNIQUE); + + new DocumentIndexWriter(nitriteConfig, indexOperations) + .updateIndexEntry(createDocument("a", 1), createDocument("a", 1), createDocument("a", 1)); + + verify(indexOperations, atLeastOnce()).buildIndex(descriptor, true); + } + + private static DocumentIndexWriter writerWithIndexOn(String field, NitriteIndexer indexer, boolean dirty) { + IndexOperations indexOperations = mock(IndexOperations.class); + IndexDescriptor descriptor = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames(field), "c"); + when(indexOperations.listIndexes()).thenReturn(new ArrayList<>(java.util.List.of(descriptor))); + when(indexOperations.shouldRebuildIndex(any())).thenReturn(dirty); + NitriteConfig nitriteConfig = mock(NitriteConfig.class); + doReturn(indexer).when(nitriteConfig).findIndexer(IndexType.NON_UNIQUE); + return new DocumentIndexWriter(nitriteConfig, indexOperations); + } +}