Keep the unclean-shutdown marker when the search index is replaced

CHANGES.md

@@ -47,6 +47,10 @@
 
 ### Bug fixes
 
+- **A search index rebuild that is interrupted is redone at the next start.**
+  When Roller replaced the index at startup it also deleted the marker that
+  records an unclean shutdown, so if it was stopped during the rebuild, the
+  next start kept the incomplete index.
 - **A bad date pattern in a theme no longer blanks the page.**
   `$utils.formatDate` logs a warning and uses the locale's standard date and
   time format. Older themes often pass a message key that no longer exists,

app/src/main/java/org/apache/roller/weblogger/business/search/lucene/LuceneIndexManager.java

@@ -131,6 +131,8 @@ public void initialize() throws InitializationException {
             if (indexConsistencyMarker.exists()) {
                 logger.debug("Index inconsistent: marker exists");
                 inconsistentAtStartup = true;
+                // deleteIndex() keeps the marker, so it stays in place until
+                // the rebuild that follows completes
                 deleteIndex();
             } else {
                 try {
@@ -382,7 +384,12 @@ private void deleteIndex() {
 
             String[] files = directory.listAll();
             for (String file : files) {
-                Files.delete(Path.of(indexDir, file));
+                // The marker says this run hasn't shut down cleanly yet. It must
+                // survive, so a crash during the rebuild that follows is caught
+                // at the next start.
+                if (!file.equals(indexConsistencyMarker.getName())) {
+                    Files.delete(Path.of(indexDir, file));
+                }
             }
         } catch (IOException ex) {
              logger.error("Problem accessing index directory", ex);

app/src/test/java/org/apache/roller/weblogger/business/search/lucene/IndexConsistencyMarkerTest.java

@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  The ASF licenses this file to You
+ * under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.  For additional information regarding
+ * copyright in this work, please see the NOTICE file in the top level
+ * directory of this distribution.
+ */
+
+package org.apache.roller.weblogger.business.search.lucene;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.roller.weblogger.business.WeblogEntryManager;
+import org.apache.roller.weblogger.business.Weblogger;
+import org.apache.roller.weblogger.business.runnable.ThreadManager;
+import org.apache.roller.weblogger.config.WebloggerConfig;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.MockedStatic;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.CALLS_REAL_METHODS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * The <code>.index-inconsistent</code> marker exists while Roller runs and is
+ * removed at a clean shutdown, so finding it at startup means the last run
+ * ended early and the index may be incomplete.
+ */
+class IndexConsistencyMarkerTest {
+
+    @TempDir
+    Path indexDir;
+
+    private LuceneIndexManager manager(ThreadManager threads) {
+        Weblogger roller = mock(Weblogger.class);
+        when(roller.getThreadManager()).thenReturn(threads);
+        when(roller.getWeblogEntryManager()).thenReturn(mock(WeblogEntryManager.class));
+        WebloggerConfig.init();
+        try (MockedStatic<WebloggerConfig> config = mockStatic(WebloggerConfig.class, CALLS_REAL_METHODS)) {
+            config.when(() -> WebloggerConfig.getProperty("search.index.dir"))
+                    .thenReturn(indexDir.toString());
+            return new LuceneIndexManager(roller);
+        }
+    }
+
+    private Path marker() {
+        return indexDir.resolve(".index-inconsistent");
+    }
+
+    @Test
+    void markerSurvivesTheIndexBeingReplaced() throws Exception {
+        // no index yet, so startup deletes what is there and rebuilds
+        ThreadManager threads = mock(ThreadManager.class);
+        LuceneIndexManager first = manager(threads);
+        first.initialize();
+        assertTrue(first.isInconsistentAtStartup());
+        verify(threads).executeInBackground(any());
+        assertTrue(Files.exists(marker()), "the marker must stay while this run is going");
+
+        // Roller is stopped before the rebuild finishes: no shutdown()
+        ThreadManager nextThreads = mock(ThreadManager.class);
+        LuceneIndexManager next = manager(nextThreads);
+        try {
+            next.initialize();
+            assertTrue(next.isInconsistentAtStartup());
+            verify(nextThreads).executeInBackground(any());
+            assertTrue(Files.exists(marker()));
+        } finally {
+            next.shutdown();
+        }
+    }
+
+    @Test
+    void cleanShutdownRemovesTheMarker() throws Exception {
+        LuceneIndexManager first = manager(mock(ThreadManager.class));
+        first.initialize();
+        first.shutdown();
+        assertFalse(Files.exists(marker()));
+
+        ThreadManager nextThreads = mock(ThreadManager.class);
+        LuceneIndexManager next = manager(nextThreads);
+        try {
+            next.initialize();
+            assertFalse(next.isInconsistentAtStartup());
+            verify(nextThreads, never()).executeInBackground(any());
+        } finally {
+            next.shutdown();
+        }
+    }
+}