From 90b47e6c430f88d61bec1a3f9e028c2c05745e83 Mon Sep 17 00:00:00 2001
From: samuel <samuel.verstraete@ontoforce.com>
Date: Tue, 3 Mar 2026 16:53:27 +0100
Subject: [PATCH 1/6] fix(backup): handle S3-style object stores in config
 restore

When restoring a backup, use both `exists()` and `listAll()` to check
for the config directory. Object stores like S3 may not have an explicit
directory marker object, but the config files are still present under
that prefix. The previous check using only `exists()` would incorrectly
throw an error in such cases.
---
 .../src/java/org/apache/solr/core/backup/BackupManager.java   | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
index 403dd589b0f9..6504d9f97ad2 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
@@ -258,7 +258,9 @@ public void uploadConfigDir(
       String sourceConfigName, String targetConfigName, ConfigSetService configSetService)
       throws IOException {
     URI source = repository.resolveDirectory(getZkStateDir(), CONFIG_STATE_DIR, sourceConfigName);
-    if (!repository.exists(source)) {
+    // Use both exists() and listAll() to handle object stores (e.g. S3) where a "directory"
+    // may have no explicit marker object but still contain config files under that prefix.
+    if (!repository.exists(source) && repository.listAll(source).length == 0) {
       throw new IllegalArgumentException("Configset expected at " + source + " does not exist");
     }
     uploadConfigToSolrCloud(configSetService, source, targetConfigName, "");

From 1f15427c232b79e8eecb5895c7f698a080aa2b55 Mon Sep 17 00:00:00 2001
From: samuel <samuel.verstraete@ontoforce.com>
Date: Tue, 3 Mar 2026 20:47:50 +0100
Subject: [PATCH 2/6] Fix S3 restore failure when directory marker objects are
 missing for configset

When a backup is copied through a local filesystem and re-uploaded to S3
(e.g. via `aws s3 sync`), the zero-byte directory marker objects created
by S3BackupRepository are typically lost. This caused restore to fail with
an error even though all configset files were present under the expected
key prefix.

Change BackupManager#uploadConfigDir to fall back to listing objects by
key prefix when the directory marker is absent, so restore succeeds as
long as the configset files themselves are present. A clear
IllegalArgumentException is still thrown when neither marker nor files
can be found.

- Add unit tests covering the no-marker, truly-absent, and normal cases
- Add documentation in the S3 backup/restore reference guide explaining
  the directory-marker behaviour and the graceful fallback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 ...ix-s3-restore-missing-directory-marker.yml |   8 +
 .../backup/BackupManagerUploadConfigTest.java | 184 ++++++++++++++++++
 .../pages/backup-restore.adoc                 |   8 +
 3 files changed, 200 insertions(+)
 create mode 100644 changelog/unreleased/fix-s3-restore-missing-directory-marker.yml
 create mode 100644 solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java

diff --git a/changelog/unreleased/fix-s3-restore-missing-directory-marker.yml b/changelog/unreleased/fix-s3-restore-missing-directory-marker.yml
new file mode 100644
index 000000000000..fbadc3d512f3
--- /dev/null
+++ b/changelog/unreleased/fix-s3-restore-missing-directory-marker.yml
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Fix restore failure when S3 backup has no directory marker objects for configset
+type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other
+authors:
+  - name: Samuel Verstraete
+links:
+  - name: GitHub PR #4182
+    url: https://github.com/apache/solr/pull/4182
diff --git a/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java b/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
new file mode 100644
index 000000000000..4fb8425b3f00
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * 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.
+ */
+package org.apache.solr.core.backup;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.net.URI;
+import org.apache.lucene.store.IOContext;
+import org.apache.lucene.store.IndexInput;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.core.ConfigSetService;
+import org.apache.solr.core.backup.repository.BackupRepository;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link BackupManager#uploadConfigDir} covering the case where an object-store
+ * backup (e.g. S3) has no explicit directory marker but the config files are still present.
+ */
+public class BackupManagerUploadConfigTest extends SolrTestCaseJ4 {
+
+  @BeforeClass
+  public static void setUpClass() {
+    assumeWorkingMockito();
+  }
+
+  private static final URI BACKUP_PATH = URI.create("s3://bucket/backup/");
+  private static final URI ZK_STATE_URI = URI.create("s3://bucket/backup/zk_backup/");
+  private static final URI CONFIG_DIR_URI =
+      URI.create("s3://bucket/backup/zk_backup/configs/myconfig/");
+  private static final URI SOLRCONFIG_URI =
+      URI.create("s3://bucket/backup/zk_backup/configs/myconfig/solrconfig.xml");
+
+  /**
+   * When the directory marker object is absent (S3 "no-marker" scenario) but config files exist
+   * under the prefix, {@code uploadConfigDir} must succeed and upload those files.
+   *
+   * <p>This is the regression test for the case where a backup was copied through a local
+   * filesystem and re-uploaded to S3, losing the zero-byte directory marker objects in the process.
+   */
+  @Test
+  public void testUploadConfigDir_withoutDirectoryMarker_succeedsWhenFilesPresent()
+      throws Exception {
+    BackupRepository mockRepo = mock(BackupRepository.class);
+    ZkStateReader mockZkStateReader = mock(ZkStateReader.class);
+    ConfigSetService mockConfigSetService = mock(ConfigSetService.class);
+
+    // Simulate S3: no marker object → exists() is false, but listAll() finds the files
+    when(mockRepo.exists(CONFIG_DIR_URI)).thenReturn(false);
+    when(mockRepo.listAll(CONFIG_DIR_URI)).thenReturn(new String[] {"solrconfig.xml"});
+
+    // resolveDirectory chain used by getZkStateDir() and uploadConfigDir()
+    when(mockRepo.resolveDirectory(BACKUP_PATH, BackupManager.ZK_STATE_DIR))
+        .thenReturn(ZK_STATE_URI);
+    when(mockRepo.resolveDirectory(ZK_STATE_URI, BackupManager.CONFIG_STATE_DIR, "myconfig"))
+        .thenReturn(CONFIG_DIR_URI);
+
+    // resolve() used by uploadConfigToSolrCloud() to build the per-file URI
+    when(mockRepo.resolve(CONFIG_DIR_URI, "solrconfig.xml")).thenReturn(SOLRCONFIG_URI);
+    when(mockRepo.getPathType(SOLRCONFIG_URI)).thenReturn(BackupRepository.PathType.FILE);
+
+    // Return minimal valid XML bytes so FileTypeMagicUtil does not reject the file
+    byte[] configBytes = "<config/>".getBytes();
+    IndexInput mockInput = mock(IndexInput.class);
+    when(mockInput.length()).thenReturn((long) configBytes.length);
+    doAnswer(
+            invocation -> {
+              byte[] dest = invocation.getArgument(0);
+              int offset = invocation.getArgument(1);
+              System.arraycopy(configBytes, 0, dest, offset, configBytes.length);
+              return null;
+            })
+        .when(mockInput)
+        .readBytes(any(), eq(0), eq(configBytes.length));
+    when(mockRepo.openInput(CONFIG_DIR_URI, "solrconfig.xml", IOContext.DEFAULT))
+        .thenReturn(mockInput);
+
+    BackupManager backupManager =
+        BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
+
+    // Must not throw even though exists() returned false
+    backupManager.uploadConfigDir("myconfig", "restoredConfig", mockConfigSetService);
+
+    // The config file must have been uploaded to the target configset
+    verify(mockConfigSetService)
+        .uploadFileToConfig(eq("restoredConfig"), eq("solrconfig.xml"), any(byte[].class), eq(false));
+  }
+
+  /**
+   * When neither a directory marker nor any files exist under the config prefix, {@code
+   * uploadConfigDir} must throw {@link IllegalArgumentException}.
+   */
+  @Test
+  public void testUploadConfigDir_throwsWhenTrulyAbsent() throws Exception {
+    BackupRepository mockRepo = mock(BackupRepository.class);
+    ZkStateReader mockZkStateReader = mock(ZkStateReader.class);
+    ConfigSetService mockConfigSetService = mock(ConfigSetService.class);
+
+    URI missingConfigDir =
+        URI.create("s3://bucket/backup/zk_backup/configs/nonexistent/");
+
+    when(mockRepo.exists(missingConfigDir)).thenReturn(false);
+    when(mockRepo.listAll(missingConfigDir)).thenReturn(new String[0]);
+
+    when(mockRepo.resolveDirectory(BACKUP_PATH, BackupManager.ZK_STATE_DIR))
+        .thenReturn(ZK_STATE_URI);
+    when(mockRepo.resolveDirectory(
+            ZK_STATE_URI, BackupManager.CONFIG_STATE_DIR, "nonexistent"))
+        .thenReturn(missingConfigDir);
+
+    BackupManager backupManager =
+        BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
+
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> backupManager.uploadConfigDir("nonexistent", "target", mockConfigSetService));
+  }
+
+  /**
+   * When the directory marker IS present (the normal case), {@code uploadConfigDir} uses the fast
+   * path via {@code exists()} and must still upload files correctly.
+   */
+  @Test
+  public void testUploadConfigDir_withDirectoryMarker_succeedsNormally() throws Exception {
+    BackupRepository mockRepo = mock(BackupRepository.class);
+    ZkStateReader mockZkStateReader = mock(ZkStateReader.class);
+    ConfigSetService mockConfigSetService = mock(ConfigSetService.class);
+
+    // Marker present → exists() returns true, listAll() should not be called
+    when(mockRepo.exists(CONFIG_DIR_URI)).thenReturn(true);
+    when(mockRepo.listAll(CONFIG_DIR_URI)).thenReturn(new String[] {"solrconfig.xml"});
+
+    when(mockRepo.resolveDirectory(BACKUP_PATH, BackupManager.ZK_STATE_DIR))
+        .thenReturn(ZK_STATE_URI);
+    when(mockRepo.resolveDirectory(ZK_STATE_URI, BackupManager.CONFIG_STATE_DIR, "myconfig"))
+        .thenReturn(CONFIG_DIR_URI);
+
+    when(mockRepo.resolve(CONFIG_DIR_URI, "solrconfig.xml")).thenReturn(SOLRCONFIG_URI);
+    when(mockRepo.getPathType(SOLRCONFIG_URI)).thenReturn(BackupRepository.PathType.FILE);
+
+    byte[] configBytes = "<config/>".getBytes();
+    IndexInput mockInput = mock(IndexInput.class);
+    when(mockInput.length()).thenReturn((long) configBytes.length);
+    doAnswer(
+            invocation -> {
+              byte[] dest = invocation.getArgument(0);
+              int offset = invocation.getArgument(1);
+              System.arraycopy(configBytes, 0, dest, offset, configBytes.length);
+              return null;
+            })
+        .when(mockInput)
+        .readBytes(any(), eq(0), eq(configBytes.length));
+    when(mockRepo.openInput(CONFIG_DIR_URI, "solrconfig.xml", IOContext.DEFAULT))
+        .thenReturn(mockInput);
+
+    BackupManager backupManager =
+        BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
+
+    backupManager.uploadConfigDir("myconfig", "restoredConfig", mockConfigSetService);
+
+    verify(mockConfigSetService)
+        .uploadFileToConfig(eq("restoredConfig"), eq("solrconfig.xml"), any(byte[].class), eq(false));
+  }
+}
diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc
index e6fa3e4d4039..5a53da952472 100644
--- a/solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc
+++ b/solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc
@@ -672,6 +672,14 @@ The location must already exist within your S3 bucket before you can perform any
 Empty Location::
 If you do not want to use a sub-directory within your bucket to store your backup, you can use any of the following location options: `/`, `s3:/`, `s3://`.
 However the location option is mandatory and you will receive an error when trying to perform backup operations without it.
+
+Directory Markers::
+S3 has no native concept of directories.
+The `S3BackupRepository` simulates directories by uploading zero-byte marker objects (e.g., `zk_backup/configs/myconfig/`) alongside the real backup files.
+If a backup is copied through a local filesystem and re-uploaded to S3 (for example using `aws s3 sync`), these zero-byte marker objects are typically lost because most tools translate them into real filesystem directories and do not recreate them on re-upload.
++
+Solr handles this gracefully: if the directory marker is absent, Solr falls back to listing objects by key prefix to confirm that the configset files are present before restoring them.
+A restore will still fail with a clear error only if neither the marker nor any configset files can be found at the expected location.
 ====
 
 An example configuration to enable S3 backups and restore can be seen below:

From 824e7f9be0345a2376fa759387e2267aa517dc0a Mon Sep 17 00:00:00 2001
From: samuel <samuel.verstraete@ontoforce.com>
Date: Tue, 3 Mar 2026 21:07:41 +0100
Subject: [PATCH 3/6] fix gradle check things

---
 .../backup/BackupManagerUploadConfigTest.java | 26 +++++++++----------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java b/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
index 4fb8425b3f00..a5c948a8aae1 100644
--- a/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
+++ b/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
@@ -24,6 +24,7 @@
 import static org.mockito.Mockito.when;
 
 import java.net.URI;
+import java.nio.charset.StandardCharsets;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
 import org.apache.solr.SolrTestCaseJ4;
@@ -80,7 +81,7 @@ public void testUploadConfigDir_withoutDirectoryMarker_succeedsWhenFilesPresent(
     when(mockRepo.getPathType(SOLRCONFIG_URI)).thenReturn(BackupRepository.PathType.FILE);
 
     // Return minimal valid XML bytes so FileTypeMagicUtil does not reject the file
-    byte[] configBytes = "<config/>".getBytes();
+    byte[] configBytes = "<config/>".getBytes(StandardCharsets.UTF_8);
     IndexInput mockInput = mock(IndexInput.class);
     when(mockInput.length()).thenReturn((long) configBytes.length);
     doAnswer(
@@ -95,15 +96,15 @@ public void testUploadConfigDir_withoutDirectoryMarker_succeedsWhenFilesPresent(
     when(mockRepo.openInput(CONFIG_DIR_URI, "solrconfig.xml", IOContext.DEFAULT))
         .thenReturn(mockInput);
 
-    BackupManager backupManager =
-        BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
+    BackupManager backupManager = BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
 
     // Must not throw even though exists() returned false
     backupManager.uploadConfigDir("myconfig", "restoredConfig", mockConfigSetService);
 
     // The config file must have been uploaded to the target configset
     verify(mockConfigSetService)
-        .uploadFileToConfig(eq("restoredConfig"), eq("solrconfig.xml"), any(byte[].class), eq(false));
+        .uploadFileToConfig(
+            eq("restoredConfig"), eq("solrconfig.xml"), any(byte[].class), eq(false));
   }
 
   /**
@@ -116,20 +117,17 @@ public void testUploadConfigDir_throwsWhenTrulyAbsent() throws Exception {
     ZkStateReader mockZkStateReader = mock(ZkStateReader.class);
     ConfigSetService mockConfigSetService = mock(ConfigSetService.class);
 
-    URI missingConfigDir =
-        URI.create("s3://bucket/backup/zk_backup/configs/nonexistent/");
+    URI missingConfigDir = URI.create("s3://bucket/backup/zk_backup/configs/nonexistent/");
 
     when(mockRepo.exists(missingConfigDir)).thenReturn(false);
     when(mockRepo.listAll(missingConfigDir)).thenReturn(new String[0]);
 
     when(mockRepo.resolveDirectory(BACKUP_PATH, BackupManager.ZK_STATE_DIR))
         .thenReturn(ZK_STATE_URI);
-    when(mockRepo.resolveDirectory(
-            ZK_STATE_URI, BackupManager.CONFIG_STATE_DIR, "nonexistent"))
+    when(mockRepo.resolveDirectory(ZK_STATE_URI, BackupManager.CONFIG_STATE_DIR, "nonexistent"))
         .thenReturn(missingConfigDir);
 
-    BackupManager backupManager =
-        BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
+    BackupManager backupManager = BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
 
     assertThrows(
         IllegalArgumentException.class,
@@ -158,7 +156,7 @@ public void testUploadConfigDir_withDirectoryMarker_succeedsNormally() throws Ex
     when(mockRepo.resolve(CONFIG_DIR_URI, "solrconfig.xml")).thenReturn(SOLRCONFIG_URI);
     when(mockRepo.getPathType(SOLRCONFIG_URI)).thenReturn(BackupRepository.PathType.FILE);
 
-    byte[] configBytes = "<config/>".getBytes();
+    byte[] configBytes = "<config/>".getBytes(StandardCharsets.UTF_8);
     IndexInput mockInput = mock(IndexInput.class);
     when(mockInput.length()).thenReturn((long) configBytes.length);
     doAnswer(
@@ -173,12 +171,12 @@ public void testUploadConfigDir_withDirectoryMarker_succeedsNormally() throws Ex
     when(mockRepo.openInput(CONFIG_DIR_URI, "solrconfig.xml", IOContext.DEFAULT))
         .thenReturn(mockInput);
 
-    BackupManager backupManager =
-        BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
+    BackupManager backupManager = BackupManager.forBackup(mockRepo, mockZkStateReader, BACKUP_PATH);
 
     backupManager.uploadConfigDir("myconfig", "restoredConfig", mockConfigSetService);
 
     verify(mockConfigSetService)
-        .uploadFileToConfig(eq("restoredConfig"), eq("solrconfig.xml"), any(byte[].class), eq(false));
+        .uploadFileToConfig(
+            eq("restoredConfig"), eq("solrconfig.xml"), any(byte[].class), eq(false));
   }
 }

From db516b54640acf0b89cd0f7409c203a3099bee95 Mon Sep 17 00:00:00 2001
From: samuel <samuel.verstraete@ontoforce.com>
Date: Fri, 6 Mar 2026 15:16:18 +0100
Subject: [PATCH 4/6] Fix S3 virtual-directory detection and stale
 collectionRefresh future

S3StorageClient.isDirectory: when no explicit directory-marker object
exists (NoSuchKeyException), fall back to listing up to one object
under the path prefix. This handles "virtual directories" that arise
after an `aws s3 sync` round-trip, which silently drops zero-byte
directory marker keys, causing restore to fail with NoSuchKey 404.

CloudSolrClient: before computeIfAbsent for collectionRefreshes, evict
any already-completed future via computeIfPresent. Without this, a
stale done future is reused instead of submitting a fresh refresh,
so callers (e.g. stale-state retry) never observe an updated ref.

Also adds a unit test covering the virtual-directory fallback path and
a changelog entry for the S3 fix.
---
 ...s3-restore-virtual-directory-detection.yml |  5 +++
 .../org/apache/solr/s3/S3StorageClient.java   | 32 +++++++++++++++++-
 .../test/org/apache/solr/s3/S3PathsTest.java  | 33 +++++++++++++++++++
 .../client/solrj/impl/CloudSolrClient.java    |  5 +++
 4 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml

diff --git a/changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml b/changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml
new file mode 100644
index 000000000000..52ba2f614120
--- /dev/null
+++ b/changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml
@@ -0,0 +1,5 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Fix S3 restore failure (NoSuchKey 404) when configset subdirectory markers were removed by aws s3 sync
+type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other
+authors:
+  - name: Samuel Verstraete
diff --git a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
index 0add42da2d6b..2f3c2a6329d7 100644
--- a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
+++ b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
@@ -315,6 +315,13 @@ boolean pathExists(String path) throws S3Exception {
   /**
    * Check if path is directory.
    *
+   * <p>A path is considered a directory if either (a) an explicit directory marker object exists
+   * (i.e. a zero-byte object with key {@code path + "/"} and content-type {@code
+   * application/x-directory}), or (b) no such marker exists but at least one object exists whose
+   * key starts with {@code path + "/"} (a "virtual directory"). Case (b) arises after an {@code aws
+   * s3 sync} round-trip, which skips directory marker objects, causing them to be absent in the
+   * target bucket.
+   *
    * @param path to File/Directory in S3.
    * @return true if path is directory, otherwise false.
    */
@@ -329,7 +336,30 @@ boolean isDirectory(String path) throws S3Exception {
       return StrUtils.isNotNullOrEmpty(contentType)
           && contentType.equalsIgnoreCase(S3_DIR_CONTENT_TYPE);
     } catch (NoSuchKeyException nske) {
-      return false;
+      // No explicit directory marker found; fall back to checking whether any objects exist under
+      // this prefix. This handles "virtual directories" that arise after an 'aws s3 sync'
+      // round-trip, which does not preserve directory marker objects (keys ending with '/').
+      return hasChildrenUnderPrefix(s3Path);
+    } catch (SdkException sdke) {
+      throw handleAmazonException(sdke);
+    }
+  }
+
+  /**
+   * Returns {@code true} if at least one object exists in S3 whose key starts with {@code dirPath}.
+   *
+   * <p>Used as a fallback in {@link #isDirectory} when no explicit directory marker object is
+   * present.
+   *
+   * @param dirPath directory path with a trailing {@code /} (as produced by {@link
+   *     #sanitizedDirPath})
+   */
+  private boolean hasChildrenUnderPrefix(String dirPath) throws S3Exception {
+    try {
+      return !s3Client
+          .listObjectsV2(b -> b.bucket(bucketName).prefix(dirPath).maxKeys(1))
+          .contents()
+          .isEmpty();
     } catch (SdkException sdke) {
       throw handleAmazonException(sdke);
     }
diff --git a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java
index d314855c3b1b..9fd1ecf9a844 100644
--- a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java
+++ b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java
@@ -163,6 +163,39 @@ public void testDeletePrefix() throws S3Exception {
     assertTrue(client.pathExists("/my-file2"));
   }
 
+  /**
+   * Test that a directory containing objects but lacking an explicit directory marker (i.e. a
+   * "virtual directory") is still recognised as a directory. This mirrors the situation that arises
+   * after an {@code aws s3 sync} round-trip: {@code aws s3 sync} skips directory marker objects
+   * (keys ending with {@code /}) when downloading, so they are never re-uploaded and are absent in
+   * the target bucket.
+   */
+  @Test
+  public void testVirtualDirectoryWithoutMarker() throws S3Exception {
+    // Set up a normal directory tree with markers and content.
+    client.createDirectory("/virtual-dir");
+    pushContent("/virtual-dir/file1", "file1");
+    client.createDirectory("/virtual-dir/sub-dir");
+    pushContent("/virtual-dir/sub-dir/file2", "file2");
+
+    // Simulate the post-sync state by deleting just the directory marker objects.
+    // deleteObjects(Collection, int) accepts raw keys, bypassing sanitizedFilePath checks.
+    client.deleteObjects(List.of("virtual-dir/", "virtual-dir/sub-dir/"), 100);
+
+    // The paths should still be considered directories via the virtual-directory fallback.
+    assertTrue(
+        "A path with objects under it should be considered a virtual directory",
+        client.isDirectory("/virtual-dir"));
+    assertTrue(
+        "A nested path with objects under it should be considered a virtual directory",
+        client.isDirectory("/virtual-dir/sub-dir"));
+
+    // A path with neither a marker nor any objects underneath must not be a directory.
+    assertFalse(
+        "A path with no objects and no marker should not be a directory",
+        client.isDirectory("/virtual-dir/empty-sub-dir"));
+  }
+
   /** Check listing objects of a directory. */
   @Test
   public void testListDir() throws S3Exception {
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
index c9199fdafff2..9631f229553b 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
@@ -1663,6 +1663,11 @@ private CompletableFuture<DocCollection> triggerCollectionRefresh(String collect
       DocCollection cached = cacheEntry == null ? null : cacheEntry.cached;
       return CompletableFuture.completedFuture(cached);
     }
+    // Remove any already-completed future that hasn't been cleaned up by whenCompleteAsync yet.
+    // Without this, computeIfAbsent below would reuse the stale done future instead of submitting
+    // a genuinely new refresh, causing callers (e.g. stale-state retry) to observe no new
+    // ref.get().
+    collectionRefreshes.computeIfPresent(collection, (k, v) -> v.isDone() ? null : v);
     return collectionRefreshes.computeIfAbsent(
         collection,
         key -> {

From 73d47c074fb6eae74926513b50d4bc5521e16f87 Mon Sep 17 00:00:00 2001
From: samuel <samuel.verstraete@ontoforce.com>
Date: Mon, 16 Mar 2026 14:37:01 +0100
Subject: [PATCH 5/6] fix(s3): remove directory marker dependency from
 backup/restore

Replace all uses of `repository.exists()` and explicit `createDirectory()`
calls with virtual-directory convention throughout the backup/restore pipeline.

S3 does not require zero-byte marker objects for directories. Writing such
markers is an anti-pattern: tools like `aws s3 sync` silently drop them,
causing subsequent `exists()` checks to return false and breaking restore.

Changes:
- `S3StorageClient.createDirectory()` is now a no-op (no marker written)
- Remove `parentDirectoryExist()` guard from `pushStream()`; files can be
  written directly under any prefix
- Replace `repository.exists()` checks with `listAllOrEmpty()` length checks
  in `BackupCmd`, `BackupManager`, and `SnapShooter`
- `BackupFilePaths.createIncrementalBackupFolders()` and
  `BackupManager.createZkStateDir()` become no-ops
- `BackupManager.downloadConfigDir()` no longer pre-creates directories
- Consolidate two duplicate changelog entries into one

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 ...ix-s3-restore-missing-directory-marker.yml |  4 +-
 ...s3-restore-virtual-directory-detection.yml |  5 --
 .../solr/cloud/api/collections/BackupCmd.java |  9 +--
 .../solr/core/backup/BackupFilePaths.java     |  9 +--
 .../solr/core/backup/BackupManager.java       | 17 ++---
 .../org/apache/solr/handler/SnapShooter.java  |  4 +-
 .../backup/BackupManagerUploadConfigTest.java |  7 +--
 .../apache/solr/s3/S3BackupRepository.java    |  1 -
 .../org/apache/solr/s3/S3StorageClient.java   | 62 +------------------
 .../test/org/apache/solr/s3/S3PathsTest.java  | 26 ++++----
 10 files changed, 33 insertions(+), 111 deletions(-)
 delete mode 100644 changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml

diff --git a/changelog/unreleased/fix-s3-restore-missing-directory-marker.yml b/changelog/unreleased/fix-s3-restore-missing-directory-marker.yml
index fbadc3d512f3..7f51fa4aff4c 100644
--- a/changelog/unreleased/fix-s3-restore-missing-directory-marker.yml
+++ b/changelog/unreleased/fix-s3-restore-missing-directory-marker.yml
@@ -1,6 +1,6 @@
 # See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
-title: Fix restore failure when S3 backup has no directory marker objects for configset
-type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other
+title: Remove S3 directory marker dependency from backup/restore — use virtual-directory convention instead
+type: fixed
 authors:
   - name: Samuel Verstraete
 links:
diff --git a/changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml b/changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml
deleted file mode 100644
index 52ba2f614120..000000000000
--- a/changelog/unreleased/fix-s3-restore-virtual-directory-detection.yml
+++ /dev/null
@@ -1,5 +0,0 @@
-# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
-title: Fix S3 restore failure (NoSuchKey 404) when configset subdirectory markers were removed by aws s3 sync
-type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other
-authors:
-  - name: Samuel Verstraete
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
index 151cbeeee1a8..3a0861cbd959 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java
@@ -198,15 +198,13 @@ private URI createAndValidateBackupPath(
       throws IOException {
     final URI backupNamePath = repository.resolveDirectory(location, backupName);
 
-    if ((!incremental) && repository.exists(backupNamePath)) {
+    if ((!incremental) && repository.listAllOrEmpty(backupNamePath).length > 0) {
       throw new SolrException(
           SolrException.ErrorCode.BAD_REQUEST,
           "The backup directory already exists: " + backupNamePath);
     }
 
-    if (!repository.exists(backupNamePath)) {
-      repository.createDirectory(backupNamePath);
-    } else if (incremental) {
+    if (incremental && repository.listAllOrEmpty(backupNamePath).length > 0) {
       final String[] directoryContents = repository.listAll(backupNamePath);
       if (directoryContents.length == 1) {
         String directoryContentsName = directoryContents[0];
@@ -238,9 +236,6 @@ private URI createAndValidateBackupPath(
     // Incremental backups have an additional directory named after the collection that needs
     // created
     final URI backupPathWithCollection = repository.resolveDirectory(backupNamePath, collection);
-    if (!repository.exists(backupPathWithCollection)) {
-      repository.createDirectory(backupPathWithCollection);
-    }
     BackupFilePaths incBackupFiles = new BackupFilePaths(repository, backupPathWithCollection);
     incBackupFiles.createIncrementalBackupFolders();
     return backupPathWithCollection;
diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java b/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
index 3e1727714ef5..2dc936ac9d5e 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
@@ -79,14 +79,15 @@ public URI getBackupLocation() {
   }
 
   /**
-   * Create all locations required to store an incremental backup.
+   * Previously created directory marker objects at the incremental backup locations. On S3-style
+   * object stores no explicit directory marker is needed; files can be written directly under any
+   * prefix. This method is kept for interface compatibility but performs no operations.
    *
    * @throws IOException for issues encountered using repository to create directories
    */
   public void createIncrementalBackupFolders() throws IOException {
-    repository.createDirectory(backupLoc);
-    repository.createDirectory(getIndexDir());
-    repository.createDirectory(getShardBackupMetadataDir());
+    // S3 does not require directory markers. On local FS, parent directories are created
+    // implicitly when files are written. No action needed.
   }
 
   /**
diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
index 6504d9f97ad2..b6ad9e613e7f 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
@@ -131,7 +131,7 @@ public static BackupManager forRestore(
     Objects.requireNonNull(repository);
     Objects.requireNonNull(stateReader);
 
-    if (!repository.exists(backupPath)) {
+    if (repository.listAllOrEmpty(backupPath).length == 0) {
       throw new SolrException(
           ErrorCode.SERVER_ERROR, "Couldn't restore since doesn't exist: " + backupPath);
     }
@@ -258,9 +258,7 @@ public void uploadConfigDir(
       String sourceConfigName, String targetConfigName, ConfigSetService configSetService)
       throws IOException {
     URI source = repository.resolveDirectory(getZkStateDir(), CONFIG_STATE_DIR, sourceConfigName);
-    // Use both exists() and listAll() to handle object stores (e.g. S3) where a "directory"
-    // may have no explicit marker object but still contain config files under that prefix.
-    if (!repository.exists(source) && repository.listAll(source).length == 0) {
+    if (repository.listAll(source).length == 0) {
       throw new IllegalArgumentException("Configset expected at " + source + " does not exist");
     }
     uploadConfigToSolrCloud(configSetService, source, targetConfigName, "");
@@ -277,9 +275,6 @@ public void uploadConfigDir(
   public void downloadConfigDir(String configName, ConfigSetService configSetService)
       throws IOException {
     URI dest = repository.resolveDirectory(getZkStateDir(), CONFIG_STATE_DIR, configName);
-    repository.createDirectory(repository.resolveDirectory(getZkStateDir(), CONFIG_STATE_DIR));
-    repository.createDirectory(dest);
-
     downloadConfigToRepo(configSetService, configName, dest);
   }
 
@@ -370,9 +365,8 @@ private void downloadConfigToRepo(ConfigSetService configSetService, String conf
           }
         }
       } else {
-        if (!repository.exists(uri)) {
-          repository.createDirectory(uri);
-        }
+        // Directory entries from getAllConfigFiles are implicit on object stores;
+        // no explicit directory creation is needed.
       }
     }
   }
@@ -433,6 +427,7 @@ private URI getZkStateDir() {
   }
 
   public void createZkStateDir() throws IOException {
-    repository.createDirectory(getZkStateDir());
+    // S3 does not require directory marker objects to be written before files are stored
+    // under a prefix. This is kept as a no-op for interface compatibility.
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
index 4fe1f0c74272..afd06ebf0f5c 100644
--- a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
+++ b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java
@@ -162,12 +162,12 @@ protected void deleteSnapAsync(final ReplicationHandler replicationHandler) {
   public void validateCreateSnapshot() throws IOException {
     // Note - Removed the current behavior of creating the directory hierarchy.
     // Do we really need to provide this support?
-    if (!backupRepo.exists(baseSnapDirPath)) {
+    if (backupRepo.listAllOrEmpty(baseSnapDirPath).length == 0) {
       throw new SolrException(
           SolrException.ErrorCode.BAD_REQUEST, " Directory does not exist: " + snapshotDirPath);
     }
 
-    if (backupRepo.exists(snapshotDirPath)) {
+    if (backupRepo.listAllOrEmpty(snapshotDirPath).length > 0) {
       throw new SolrException(
           SolrException.ErrorCode.BAD_REQUEST,
           "Snapshot directory already exists: " + snapshotDirPath);
diff --git a/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java b/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
index a5c948a8aae1..cf7c1d739d92 100644
--- a/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
+++ b/solr/core/src/test/org/apache/solr/core/backup/BackupManagerUploadConfigTest.java
@@ -66,8 +66,7 @@ public void testUploadConfigDir_withoutDirectoryMarker_succeedsWhenFilesPresent(
     ZkStateReader mockZkStateReader = mock(ZkStateReader.class);
     ConfigSetService mockConfigSetService = mock(ConfigSetService.class);
 
-    // Simulate S3: no marker object → exists() is false, but listAll() finds the files
-    when(mockRepo.exists(CONFIG_DIR_URI)).thenReturn(false);
+    // Simulate S3: no marker object → listAll() finds the files
     when(mockRepo.listAll(CONFIG_DIR_URI)).thenReturn(new String[] {"solrconfig.xml"});
 
     // resolveDirectory chain used by getZkStateDir() and uploadConfigDir()
@@ -119,7 +118,6 @@ public void testUploadConfigDir_throwsWhenTrulyAbsent() throws Exception {
 
     URI missingConfigDir = URI.create("s3://bucket/backup/zk_backup/configs/nonexistent/");
 
-    when(mockRepo.exists(missingConfigDir)).thenReturn(false);
     when(mockRepo.listAll(missingConfigDir)).thenReturn(new String[0]);
 
     when(mockRepo.resolveDirectory(BACKUP_PATH, BackupManager.ZK_STATE_DIR))
@@ -144,8 +142,7 @@ public void testUploadConfigDir_withDirectoryMarker_succeedsNormally() throws Ex
     ZkStateReader mockZkStateReader = mock(ZkStateReader.class);
     ConfigSetService mockConfigSetService = mock(ConfigSetService.class);
 
-    // Marker present → exists() returns true, listAll() should not be called
-    when(mockRepo.exists(CONFIG_DIR_URI)).thenReturn(true);
+    // listAll() is the sole existence check
     when(mockRepo.listAll(CONFIG_DIR_URI)).thenReturn(new String[] {"solrconfig.xml"});
 
     when(mockRepo.resolveDirectory(BACKUP_PATH, BackupManager.ZK_STATE_DIR))
diff --git a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepository.java b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepository.java
index 23b1ed4c509a..07858b29095c 100644
--- a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepository.java
+++ b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepository.java
@@ -285,7 +285,6 @@ public void copyIndexFileFrom(
         throw new CorruptIndexException("file is too small:" + indexInput.length(), indexInput);
       }
 
-      client.createDirectory(getS3Path(dest));
       try (OutputStream outputStream = client.pushStream(s3Path)) {
 
         byte[] buffer = new byte[CHUNK_SIZE];
diff --git a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
index 2f3c2a6329d7..0ae5cce44a19 100644
--- a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
+++ b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
@@ -39,10 +39,8 @@
 import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider;
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
 import software.amazon.awssdk.awscore.retry.AwsRetryStrategy;
-import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.core.exception.SdkException;
 import software.amazon.awssdk.core.retry.RetryMode;
-import software.amazon.awssdk.core.sync.RequestBody;
 import software.amazon.awssdk.core.sync.ResponseTransformer;
 import software.amazon.awssdk.http.apache.ApacheHttpClient;
 import software.amazon.awssdk.http.apache.ProxyConfiguration;
@@ -61,7 +59,6 @@
 import software.amazon.awssdk.services.s3.model.NoSuchBucketException;
 import software.amazon.awssdk.services.s3.model.NoSuchKeyException;
 import software.amazon.awssdk.services.s3.model.ObjectIdentifier;
-import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.services.s3.model.S3Object;
 import software.amazon.awssdk.services.s3.paginators.ListObjectsV2Iterable;
 
@@ -181,27 +178,9 @@ private static S3Client createInternalClient(
 
   /** Create a directory in S3, if it does not already exist. */
   void createDirectory(String path) throws S3Exception {
-    String sanitizedDirPath = sanitizedDirPath(path);
-
-    // Only create the directory if it does not already exist
-    if (!pathExists(sanitizedDirPath)) {
-      createDirectory(getParentDirectory(sanitizedDirPath));
-      // TODO see https://issues.apache.org/jira/browse/SOLR-15359
-      //            throw new S3Exception("Parent directory doesn't exist, path=" + path);
-
-      try {
-        // Create empty object with content type header
-        PutObjectRequest putRequest =
-            PutObjectRequest.builder()
-                .bucket(bucketName)
-                .contentType(S3_DIR_CONTENT_TYPE)
-                .key(sanitizedDirPath)
-                .build();
-        s3Client.putObject(putRequest, RequestBody.empty());
-      } catch (SdkClientException ase) {
-        throw handleAmazonException(ase);
-      }
-    }
+    // S3 does not require explicit directory marker objects. Writing a file directly
+    // under a prefix is sufficient. Creating zero-byte marker objects is an anti-pattern
+    // that causes failures after tools like 'aws s3 sync' drop them.
   }
 
   /**
@@ -236,9 +215,6 @@ void deleteDirectory(String path) throws S3Exception {
 
     // Get all the files and subdirectories
     Set<String> entries = listAll(path);
-    if (pathExists(path)) {
-      entries.add(path);
-    }
 
     deleteObjects(entries);
   }
@@ -435,10 +411,6 @@ InputStream pullStream(String path) throws S3Exception {
   OutputStream pushStream(String path) throws S3Exception {
     path = sanitizedFilePath(path);
 
-    if (!parentDirectoryExist(path)) {
-      throw new S3Exception("Parent directory doesn't exist of path: " + path);
-    }
-
     try {
       return new S3OutputStream(s3Client, path, bucketName);
     } catch (SdkException sdke) {
@@ -545,34 +517,6 @@ private Set<String> listAll(String path) throws S3Exception {
     }
   }
 
-  private boolean parentDirectoryExist(String path) throws S3Exception {
-    // Get the last non-slash character of the string, to find the parent directory
-    String parentDirectory = getParentDirectory(path);
-
-    // If we have no specific parent directory, we consider parent is root (and always exists)
-    if (parentDirectory.isEmpty() || parentDirectory.equals(S3_FILE_PATH_DELIMITER)) {
-      return true;
-    }
-
-    // Check for existence twice, because s3Mock has issues in the tests
-    return pathExists(parentDirectory);
-  }
-
-  private String getParentDirectory(String path) {
-    if (!path.contains(S3_FILE_PATH_DELIMITER)) {
-      return "";
-    }
-
-    // Get the last non-slash character of the string, to find the parent directory
-    int fromEnd = path.length() - 1;
-    if (path.endsWith(S3_FILE_PATH_DELIMITER)) {
-      fromEnd -= 1;
-    }
-    return fromEnd > 0
-        ? path.substring(0, path.lastIndexOf(S3_FILE_PATH_DELIMITER, fromEnd) + 1)
-        : "";
-  }
-
   /** Ensures path adheres to some rules: -Doesn't start with a leading slash */
   String sanitizedPath(String path) throws S3Exception {
     // Trim space from start and end
diff --git a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java
index 9fd1ecf9a844..e774d29434bf 100644
--- a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java
+++ b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3PathsTest.java
@@ -46,13 +46,17 @@ public void testFiles() throws S3Exception {
   /** Simple tests with a directory. */
   @Test
   public void testDirectory() throws S3Exception {
+    // createDirectory is a no-op on S3 (no marker objects are written).
+    // A prefix is treated as a directory if it has children — write one to establish the prefix.
+    client.createDirectory("/simple-directory"); // no-op
+    pushContent("/simple-directory/a-file", "content");
 
-    client.createDirectory("/simple-directory");
-    assertFalse("Dir path needs a trailing slash", client.pathExists("/simple-directory"));
-    assertTrue("Dir should exist without a leading slash", client.pathExists("simple-directory/"));
-    assertTrue("Dir should exist with a leading slash", client.pathExists("/simple-directory/"));
+    assertFalse(
+        "Bare name without trailing slash is not a key", client.pathExists("/simple-directory"));
     assertTrue(
-        "Leading slash should be irrelevant for determining if dir is a dir",
+        "Dir should exist without a leading slash", client.pathExists("simple-directory/a-file"));
+    assertTrue(
+        "Prefix with children should be considered a directory",
         client.isDirectory("simple-directory/"));
     assertTrue(
         "Leading slash should be irrelevant for determining if dir is a dir",
@@ -172,25 +176,17 @@ public void testDeletePrefix() throws S3Exception {
    */
   @Test
   public void testVirtualDirectoryWithoutMarker() throws S3Exception {
-    // Set up a normal directory tree with markers and content.
-    client.createDirectory("/virtual-dir");
+    // Since createDirectory is a no-op, there are never marker objects.
+    // Write files directly under a prefix to establish virtual directories.
     pushContent("/virtual-dir/file1", "file1");
-    client.createDirectory("/virtual-dir/sub-dir");
     pushContent("/virtual-dir/sub-dir/file2", "file2");
 
-    // Simulate the post-sync state by deleting just the directory marker objects.
-    // deleteObjects(Collection, int) accepts raw keys, bypassing sanitizedFilePath checks.
-    client.deleteObjects(List.of("virtual-dir/", "virtual-dir/sub-dir/"), 100);
-
-    // The paths should still be considered directories via the virtual-directory fallback.
     assertTrue(
         "A path with objects under it should be considered a virtual directory",
         client.isDirectory("/virtual-dir"));
     assertTrue(
         "A nested path with objects under it should be considered a virtual directory",
         client.isDirectory("/virtual-dir/sub-dir"));
-
-    // A path with neither a marker nor any objects underneath must not be a directory.
     assertFalse(
         "A path with no objects and no marker should not be a directory",
         client.isDirectory("/virtual-dir/empty-sub-dir"));

From 91eb3cb43b2426883da6a58ddb2006ea29ec3086 Mon Sep 17 00:00:00 2001
From: samuel <samuel.verstraete@ontoforce.com>
Date: Wed, 18 Mar 2026 13:26:57 +0100
Subject: [PATCH 6/6] test(s3-repository): update tests to reflect markerless
 directory behavior
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

S3 does not use explicit directory markers, so `exists()` on a directory
URI returns false even after `createDirectory()` — a directory is only
implied by the objects it contains.

- Update S3BackupRepositoryTest assertions to expect `false` from
  `exists()` on directories, and verify content is still listable
- Override `testCanDetermineWhetherFilesAndDirectoriesExist`,
  `testCanDistinguishBetweenFilesAndDirectories`,
  `testCanDeleteEmptyOrFullDirectories`, and `testCanListFullOrEmptyDirectories`
  with S3-specific expectations (no directory marker objects)
- Add `testSkipConfigset` override in S3IncrementalBackupTest to assert
  no explicit directory entries are created during S3 backup, while still
  verifying configset files are present/absent based on the backup flag
- Fix S3ReadWriteTest#testDirectoryLength to push a child file into the
  directory before testing, satisfying S3's implicit directory semantics

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../solr/s3/S3BackupRepositoryTest.java       | 150 +++++++++++++++++-
 .../solr/s3/S3IncrementalBackupTest.java      | 119 ++++++++++++++
 .../org/apache/solr/s3/S3ReadWriteTest.java   |   1 +
 3 files changed, 265 insertions(+), 5 deletions(-)

diff --git a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java
index 18e30d194388..c2f35639c771 100644
--- a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java
+++ b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java
@@ -90,7 +90,13 @@ public void testURI() throws IOException {
           directoryUri.toString());
 
       repo.createDirectory(directoryUri);
-      assertTrue(repo.exists(directoryUri));
+      assertFalse(repo.exists(directoryUri));
+      try (OutputStream os =
+          repo.createOutput(repo.resolve(directoryUri, "markerless-child.txt"))) {
+        os.write(1);
+      }
+      assertFalse(repo.exists(directoryUri));
+      assertEquals(1, repo.listAll(directoryUri).length);
       directoryUri = repo.createDirectoryURI("d/");
       assertEquals(
           "createDirectoryURI should have a single trailing slash, even if one is provided",
@@ -110,16 +116,22 @@ public void testLocalDirectoryFunctions() throws Exception {
 
       URI path = new URI("/test/");
       repo.createDirectory(path);
-      assertTrue(repo.exists(path));
-      assertEquals(BackupRepository.PathType.DIRECTORY, repo.getPathType(path));
+      assertFalse(repo.exists(path));
       assertEquals("No files should exist in dir yet", repo.listAll(path).length, 0);
 
       URI subDir = new URI("/test/dir/");
       repo.createDirectory(subDir);
-      assertTrue(repo.exists(subDir));
-      assertEquals(BackupRepository.PathType.DIRECTORY, repo.getPathType(subDir));
+      assertFalse(repo.exists(subDir));
       assertEquals("No files should exist in subdir yet", repo.listAll(subDir).length, 0);
 
+      try (OutputStream os = repo.createOutput(new URI("/test/dir/file.txt"))) {
+        os.write(1);
+      }
+      assertFalse(repo.exists(path));
+      assertFalse(repo.exists(subDir));
+      assertEquals(BackupRepository.PathType.DIRECTORY, repo.getPathType(path));
+      assertEquals(BackupRepository.PathType.DIRECTORY, repo.getPathType(subDir));
+
       assertEquals(
           "subDir should now be returned when listing all in parent dir",
           repo.listAll(path).length,
@@ -246,6 +258,134 @@ public void testRandomAccessInput() throws Exception {
     doRandomAccessTest(content, content.indexOf("content"));
   }
 
+  @Override
+  @Test
+  public void testCanDetermineWhetherFilesAndDirectoriesExist() throws Exception {
+    try (BackupRepository repo = getRepository()) {
+      final URI emptyDirUri = repo.resolve(getBaseUri(), "emptyDir/");
+      final URI nonEmptyDirUri = repo.resolve(getBaseUri(), "nonEmptyDir/");
+      final URI nestedFileUri = repo.resolve(nonEmptyDirUri, "file.txt");
+      repo.createDirectory(emptyDirUri);
+      repo.createDirectory(nonEmptyDirUri);
+      try (OutputStream os = repo.createOutput(nestedFileUri)) {
+        os.write(100);
+      }
+
+      assertFalse(repo.exists(emptyDirUri));
+      assertFalse(repo.exists(nonEmptyDirUri));
+      assertEquals(1, repo.listAll(nonEmptyDirUri).length);
+      assertTrue(repo.exists(nestedFileUri));
+      final URI nonexistedDirUri = repo.resolve(getBaseUri(), "nonexistentDir/");
+      assertFalse(repo.exists(nonexistedDirUri));
+    }
+  }
+
+  @Override
+  @Test
+  public void testCanDistinguishBetweenFilesAndDirectories() throws Exception {
+    try (BackupRepository repo = getRepository()) {
+      final URI nonEmptyDirUri = repo.resolve(getBaseUri(), "nonEmptyDir/");
+      final URI nestedFileUri = repo.resolve(nonEmptyDirUri, "file.txt");
+      repo.createDirectory(nonEmptyDirUri);
+      try (OutputStream os = repo.createOutput(nestedFileUri)) {
+        os.write(100);
+      }
+
+      assertEquals(BackupRepository.PathType.DIRECTORY, repo.getPathType(nonEmptyDirUri));
+      assertEquals(BackupRepository.PathType.FILE, repo.getPathType(nestedFileUri));
+    }
+  }
+
+  @Override
+  @Test
+  public void testCanDeleteEmptyOrFullDirectories() throws Exception {
+    try (BackupRepository repo = getRepository()) {
+      final URI emptyDirUri = repo.resolve(getBaseUri(), "emptyDir");
+      final URI nonEmptyDirUri = repo.resolve(getBaseUri(), "nonEmptyDir");
+      final URI fileUri = repo.resolve(nonEmptyDirUri, "file.txt");
+      repo.createDirectory(emptyDirUri);
+      repo.createDirectory(nonEmptyDirUri);
+      try (OutputStream os = repo.createOutput(fileUri)) {
+        os.write(100);
+      }
+      repo.deleteDirectory(emptyDirUri);
+      repo.deleteDirectory(nonEmptyDirUri);
+      assertFalse(repo.exists(emptyDirUri));
+      assertFalse(repo.exists(nonEmptyDirUri));
+      assertFalse(repo.exists(fileUri));
+
+      final URI level1DeeplyNestedUri = repo.resolve(getBaseUri(), "nest1/");
+      final URI level2DeeplyNestedUri = repo.resolve(level1DeeplyNestedUri, "nest2/");
+      final URI level3DeeplyNestedUri = repo.resolve(level2DeeplyNestedUri, "nest3/");
+      final URI level4DeeplyNestedUri = repo.resolve(level3DeeplyNestedUri, "nest4/");
+      final URI keepFileUri = repo.resolve(level2DeeplyNestedUri, "keep.txt");
+      final URI nestedFileUri = repo.resolve(level4DeeplyNestedUri, "file.txt");
+      repo.createDirectory(level1DeeplyNestedUri);
+      repo.createDirectory(level2DeeplyNestedUri);
+      repo.createDirectory(level3DeeplyNestedUri);
+      repo.createDirectory(level4DeeplyNestedUri);
+      try (OutputStream os = repo.createOutput(keepFileUri)) {
+        os.write(100);
+      }
+      try (OutputStream os = repo.createOutput(nestedFileUri)) {
+        os.write(101);
+      }
+
+      repo.deleteDirectory(level3DeeplyNestedUri);
+      assertFalse(repo.exists(level1DeeplyNestedUri));
+      assertFalse(repo.exists(level2DeeplyNestedUri));
+      assertEquals(1, repo.listAll(level2DeeplyNestedUri).length);
+      assertFalse(repo.exists(level3DeeplyNestedUri));
+      assertFalse(repo.exists(level4DeeplyNestedUri));
+    }
+  }
+
+  @Override
+  @Test
+  public void testCanListFullOrEmptyDirectories() throws Exception {
+    try (BackupRepository repo = getRepository()) {
+      final URI rootUri = repo.resolve(getBaseUri(), "containsOtherDirs");
+      final URI otherDir1Uri = repo.resolve(rootUri, "otherDir1");
+      final URI otherDir2Uri = repo.resolve(rootUri, "otherDir2");
+      final URI otherDir3Uri = repo.resolve(rootUri, "otherDir3");
+      final URI file1Uri = repo.resolve(otherDir3Uri, "file1.txt");
+      final URI file2Uri = repo.resolve(otherDir3Uri, "file2.txt");
+      repo.createDirectory(rootUri);
+      repo.createDirectory(otherDir1Uri);
+      repo.createDirectory(otherDir2Uri);
+      repo.createDirectory(otherDir3Uri);
+      try (OutputStream os = repo.createOutput(repo.resolve(otherDir1Uri, "a.txt"))) {
+        os.write(100);
+      }
+      try (OutputStream os = repo.createOutput(repo.resolve(otherDir2Uri, "b.txt"))) {
+        os.write(101);
+      }
+      try (OutputStream os = repo.createOutput(file1Uri)) {
+        os.write(102);
+      }
+      try (OutputStream os = repo.createOutput(file2Uri)) {
+        os.write(103);
+      }
+
+      final java.util.List<String> rootChildren = java.util.List.of(repo.listAll(rootUri));
+      assertEquals(3, rootChildren.size());
+      assertTrue(rootChildren.contains("otherDir1"));
+      assertTrue(rootChildren.contains("otherDir2"));
+      assertTrue(rootChildren.contains("otherDir3"));
+
+      final java.util.List<String> otherDir2Children =
+          java.util.List.of(repo.listAll(otherDir2Uri));
+      assertEquals(1, otherDir2Children.size());
+      assertTrue(otherDir2Children.contains("b.txt"));
+
+      final java.util.List<String> otherDir3Children =
+          java.util.List.of(repo.listAll(otherDir3Uri));
+      assertEquals(2, otherDir3Children.size());
+      assertTrue(otherDir3Children.contains("file1.txt"));
+      assertTrue(otherDir3Children.contains("file2.txt"));
+    }
+  }
+
   /**
    * Check implementation of {@link S3BackupRepository#openInput(URI, String, IOContext)}. Open an
    * index input and seek to an absolute position.
diff --git a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3IncrementalBackupTest.java b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3IncrementalBackupTest.java
index 80c5207505b1..bcc4ca934936 100644
--- a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3IncrementalBackupTest.java
+++ b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3IncrementalBackupTest.java
@@ -20,10 +20,19 @@
 import com.adobe.testing.s3mock.junit4.S3MockRule;
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
 import java.lang.invoke.MethodHandles;
+import java.util.stream.Collectors;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.SolrQuery;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
 import org.apache.lucene.tests.util.LuceneTestCase;
 import org.apache.solr.cloud.api.collections.AbstractIncrementalBackupTest;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.core.TrackingBackupRepository;
+import org.apache.solr.core.backup.repository.BackupRepository;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
+import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import software.amazon.awssdk.regions.Region;
@@ -122,4 +131,114 @@ public String getCollectionNamePrefix() {
   public String getBackupLocation() {
     return "/";
   }
+
+    @Override
+    @Test
+    public void testSkipConfigset() throws Exception {
+    setTestSuffix("testskipconfigset");
+    final String backupCollectionName = getCollectionName();
+    final String restoreCollectionName = backupCollectionName + "-restore";
+
+    CloudSolrClient solrClient = cluster.getSolrClient();
+
+    CollectionAdminRequest.createCollection(backupCollectionName, "conf1", NUM_SHARDS, 1)
+      .process(solrClient);
+    int numDocs = indexDocs(backupCollectionName, true);
+    String backupName = BACKUPNAME_PREFIX + testSuffix;
+    try (BackupRepository repository =
+      cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME)) {
+      String backupLocation = repository.getBackupLocation(getBackupLocation());
+      CollectionAdminRequest.backupCollection(backupCollectionName, backupName)
+        .setLocation(backupLocation)
+        .setBackupConfigset(false)
+        .setRepositoryName(BACKUP_REPO_NAME)
+        .processAndWait(cluster.getSolrClient(), 100);
+
+      assertFalse(
+        "Configset shouldn't be part of the backup but found:\n"
+          + TrackingBackupRepository.directoriesCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.directoriesCreated().stream()
+          .anyMatch(f -> f.getPath().contains("configs/conf1")));
+      assertFalse(
+        "Configset shouldn't be part of the backup but found:\n"
+          + TrackingBackupRepository.outputsCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.outputsCreated().stream()
+          .anyMatch(f -> f.getPath().contains("configs/conf1")));
+      assertFalse(
+        "solrconfig.xml shouldn't be part of the backup but found:\n"
+          + TrackingBackupRepository.outputsCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.outputsCreated().stream()
+          .anyMatch(f -> f.getPath().contains("solrconfig.xml")));
+      assertFalse(
+        "schema.xml shouldn't be part of the backup but found:\n"
+          + TrackingBackupRepository.outputsCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.outputsCreated().stream()
+          .anyMatch(f -> f.getPath().contains("schema.xml")));
+
+      CollectionAdminRequest.restoreCollection(restoreCollectionName, backupName)
+        .setLocation(backupLocation)
+        .setRepositoryName(BACKUP_REPO_NAME)
+        .processAndWait(solrClient, 500);
+
+      AbstractFullDistribZkTestBase.waitForRecoveriesToFinish(
+        restoreCollectionName, ZkStateReader.from(solrClient), log.isDebugEnabled(), false, 3);
+      assertEquals(
+        numDocs,
+        cluster
+          .getSolrClient()
+          .query(restoreCollectionName, new SolrQuery("*:*"))
+          .getResults()
+          .getNumFound());
+    }
+
+    TrackingBackupRepository.clear();
+
+    try (BackupRepository repository =
+      cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME)) {
+      String backupLocation = repository.getBackupLocation(getBackupLocation());
+      CollectionAdminRequest.backupCollection(backupCollectionName, backupName)
+        .setLocation(backupLocation)
+        .setBackupConfigset(true)
+        .setRepositoryName(BACKUP_REPO_NAME)
+        .processAndWait(cluster.getSolrClient(), 100);
+
+      // S3 backups should never create explicit directory markers.
+      assertFalse(
+        "Configset directory creation should not happen for S3 backups:\n"
+          + TrackingBackupRepository.directoriesCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.directoriesCreated().stream()
+          .anyMatch(f -> f.getPath().contains("configs/conf1")));
+      assertTrue(
+        "Configset should be part of the backup but not found:\n"
+          + TrackingBackupRepository.outputsCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.outputsCreated().stream()
+          .anyMatch(f -> f.getPath().contains("configs/conf1")));
+      assertTrue(
+        "solrconfig.xml should be part of the backup but not found:\n"
+          + TrackingBackupRepository.outputsCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.outputsCreated().stream()
+          .anyMatch(f -> f.getPath().contains("solrconfig.xml")));
+      assertTrue(
+        "schema.xml should be part of the backup but not found:\n"
+          + TrackingBackupRepository.outputsCreated().stream()
+            .map(java.net.URI::toString)
+            .collect(Collectors.joining("\n")),
+        TrackingBackupRepository.outputsCreated().stream()
+          .anyMatch(f -> f.getPath().contains("schema.xml")));
+    }
+    }
 }
diff --git a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3ReadWriteTest.java b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3ReadWriteTest.java
index d4d13c717580..3e18f7cc31c2 100644
--- a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3ReadWriteTest.java
+++ b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3ReadWriteTest.java
@@ -90,6 +90,7 @@ public void testLength() throws Exception {
   @Test
   public void testDirectoryLength() throws Exception {
     client.createDirectory("/directory");
+    pushContent("/directory/file", "child");
 
     S3Exception exception =
         assertThrows(