Fix file settings service tests

muted-tests.yml

@@ -238,9 +238,6 @@ tests:
 - class: org.elasticsearch.xpack.inference.DefaultEndPointsIT
   method: testInferDeploysDefaultE5
   issue: https://github.com/elastic/elasticsearch/issues/115361
-- class: org.elasticsearch.reservedstate.service.FileSettingsServiceTests
-  method: testProcessFileChanges
-  issue: https://github.com/elastic/elasticsearch/issues/115280
 - class: org.elasticsearch.xpack.security.FileSettingsRoleMappingsRestartIT
   method: testFileSettingsReprocessedOnRestartWithoutVersionChange
   issue: https://github.com/elastic/elasticsearch/issues/115450

server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java

@@ -24,6 +24,8 @@
 import org.elasticsearch.common.component.Lifecycle;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.core.IOUtils;
+import org.elasticsearch.core.Strings;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.env.BuildVersion;
 import org.elasticsearch.env.Environment;
@@ -39,9 +41,10 @@
 import org.mockito.stubbing.Answer;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -50,6 +53,8 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
 
+import static java.nio.file.StandardCopyOption.ATOMIC_MOVE;
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
 import static org.hamcrest.Matchers.anEmptyMap;
 import static org.hamcrest.Matchers.hasEntry;
@@ -190,9 +195,7 @@ public void testInitialFileWorks() throws Exception {
             return null;
         }).when(controller).process(any(), any(XContentParser.class), any(), any());
 
-        CountDownLatch latch = new CountDownLatch(1);
-
-        fileSettingsService.addFileChangedListener(latch::countDown);
+        CountDownLatch fileProcessingLatch = new CountDownLatch(1);
 
         Files.createDirectories(fileSettingsService.watchedFileDir());
         // contents of the JSON don't matter, we just need a file to exist
@@ -202,15 +205,14 @@ public void testInitialFileWorks() throws Exception {
             try {
                 return invocation.callRealMethod();
             } finally {
-                latch.countDown();
+                fileProcessingLatch.countDown();
             }
         }).when(fileSettingsService).processFileOnServiceStart();
 
         fileSettingsService.start();
         fileSettingsService.clusterChanged(new ClusterChangedEvent("test", clusterService.state(), ClusterState.EMPTY_STATE));
 
-        // wait for listener to be called
-        assertTrue(latch.await(20, TimeUnit.SECONDS));
+        longAwait(fileProcessingLatch);
 
         verify(fileSettingsService, times(1)).processFileOnServiceStart();
         verify(controller, times(1)).process(any(), any(XContentParser.class), eq(ReservedStateVersionCheck.HIGHER_OR_SAME_VERSION), any());
@@ -223,40 +225,40 @@ public void testProcessFileChanges() throws Exception {
             return null;
         }).when(controller).process(any(), any(XContentParser.class), any(), any());
 
-        // we get three events: initial clusterChanged event, first write, second write
-        CountDownLatch latch = new CountDownLatch(3);
-
-        fileSettingsService.addFileChangedListener(latch::countDown);
-
-        Files.createDirectories(fileSettingsService.watchedFileDir());
-        // contents of the JSON don't matter, we just need a file to exist
-        writeTestFile(fileSettingsService.watchedFile(), "{}");
-
+        CountDownLatch changesOnStartLatch = new CountDownLatch(1);
         doAnswer((Answer<?>) invocation -> {
             try {
                 return invocation.callRealMethod();
             } finally {
-                latch.countDown();
+                changesOnStartLatch.countDown();
             }
         }).when(fileSettingsService).processFileOnServiceStart();
+
+        CountDownLatch changesLatch = new CountDownLatch(1);
         doAnswer((Answer<?>) invocation -> {
             try {
                 return invocation.callRealMethod();
             } finally {
-                latch.countDown();
+                changesLatch.countDown();
             }
         }).when(fileSettingsService).processFileChanges();
 
+        Files.createDirectories(fileSettingsService.watchedFileDir());
+        // contents of the JSON don't matter, we just need a file to exist
+        writeTestFile(fileSettingsService.watchedFile(), "{}");
+
         fileSettingsService.start();
         fileSettingsService.clusterChanged(new ClusterChangedEvent("test", clusterService.state(), ClusterState.EMPTY_STATE));
-        // second file change; contents still don't matter
-        overwriteTestFile(fileSettingsService.watchedFile(), "{}");
 
-        // wait for listener to be called (once for initial processing, once for subsequent update)
-        assertTrue(latch.await(20, TimeUnit.SECONDS));
+        longAwait(changesOnStartLatch);
 
         verify(fileSettingsService, times(1)).processFileOnServiceStart();
         verify(controller, times(1)).process(any(), any(XContentParser.class), eq(ReservedStateVersionCheck.HIGHER_OR_SAME_VERSION), any());
+
+        // second file change; contents still don't matter
+        writeTestFile(fileSettingsService.watchedFile(), "[]");
+        longAwait(changesLatch);
+
         verify(fileSettingsService, times(1)).processFileChanges();
         verify(controller, times(1)).process(any(), any(XContentParser.class), eq(ReservedStateVersionCheck.HIGHER_VERSION_ONLY), any());
     }
@@ -295,9 +297,7 @@ public void testStopWorksInMiddleOfProcessing() throws Exception {
         // Make some fake settings file to cause the file settings service to process it
         writeTestFile(fileSettingsService.watchedFile(), "{}");
 
-        // we need to wait a bit, on MacOS it may take up to 10 seconds for the Java watcher service to notice the file,
-        // on Linux is instantaneous. Windows is instantaneous too.
-        assertTrue(processFileLatch.await(30, TimeUnit.SECONDS));
+        longAwait(processFileLatch);
 
         // Stopping the service should interrupt the watcher thread, we should be able to stop
         fileSettingsService.stop();
@@ -352,15 +352,34 @@ public void testHandleSnapshotRestoreResetsMetadata() throws Exception {
     }
 
     // helpers
-    private void writeTestFile(Path path, String contents) throws IOException {
-        Path tempFilePath = createTempFile();
-        Files.writeString(tempFilePath, contents);
-        Files.move(tempFilePath, path, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
+    private static void writeTestFile(Path path, String contents) {
+        Path tempFile = null;
+        try {
+            tempFile = Files.createTempFile(path.getParent(), path.getFileName().toString(), "tmp");
+            Files.writeString(tempFile, contents);
+
+            try {
+                Files.move(tempFile, path, REPLACE_EXISTING, ATOMIC_MOVE);
+            } catch (AtomicMoveNotSupportedException e) {
+                Files.move(tempFile, path, REPLACE_EXISTING);
+            }
+        } catch (final IOException e) {
+            throw new UncheckedIOException(Strings.format("could not write file [%s]", path.toAbsolutePath()), e);
+        } finally {
+            // we are ignoring exceptions here, so we do not need handle whether or not tempFile was initialized nor if the file exists
+            IOUtils.deleteFilesIgnoringExceptions(tempFile);
+        }
     }
 
-    private void overwriteTestFile(Path path, String contents) throws IOException {
-        Path tempFilePath = createTempFile();
-        Files.writeString(tempFilePath, contents);
-        Files.move(tempFilePath, path, StandardCopyOption.REPLACE_EXISTING);
+    // this waits for up to 20 seconds to account for watcher service differences between OSes
+    // on MacOS it may take up to 10 seconds for the Java watcher service to notice the file,
+    // on Linux is instantaneous. Windows is instantaneous too.
+    private static void longAwait(CountDownLatch latch) {
+        try {
+            assertTrue("longAwait: CountDownLatch did not reach zero within the timeout", latch.await(20, TimeUnit.SECONDS));
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            fail(e, "longAwait: interrupted waiting for CountDownLatch to reach zero");
+        }
     }
 }