This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 82fccd08092 Review and tidy solr/modules/s3-repository code (#4838)
82fccd08092 is described below
commit 82fccd0809255bbbbb64b1fffc9abd7cb353fabd
Author: Eric Pugh <[email protected]>
AuthorDate: Thu Sep 17 14:15:25 2026 -0400
Review and tidy solr/modules/s3-repository code (#4838)
---
.../org/apache/solr/s3/S3BackupRepositoryConfig.java | 19 -------------------
.../src/java/org/apache/solr/s3/S3OutputStream.java | 2 +-
.../src/java/org/apache/solr/s3/S3StorageClient.java | 12 ++++++------
.../org/apache/solr/s3/S3BackupRepositoryTest.java | 12 ++++++------
.../org/apache/solr/s3/S3IncrementalBackupTest.java | 6 ------
.../test/org/apache/solr/s3/S3OutputStreamTest.java | 2 +-
.../src/test/org/apache/solr/s3/S3ReadWriteTest.java | 4 ++--
7 files changed, 16 insertions(+), 41 deletions(-)
diff --git
a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepositoryConfig.java
b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepositoryConfig.java
index 43d6279bc94..06cc54f6795 100644
---
a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepositoryConfig.java
+++
b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3BackupRepositoryConfig.java
@@ -67,25 +67,6 @@ public class S3BackupRepositoryConfig {
}
}
- static int getIntConfig(NamedList<?> config, String property) {
- return getIntConfig(config, property, 0);
- }
-
- static int getIntConfig(NamedList<?> config, String property, int def) {
- String envProp = EnvUtils.getProperty(property);
- if (envProp == null) {
- Object configProp = config.get(property);
- return configProp instanceof Integer ? (int) configProp : def;
- } else {
- return Integer.parseInt(envProp);
- }
- }
-
- /** If the property as any other value than 'true' or 'TRUE', this will
default to false. */
- static boolean getBooleanConfig(NamedList<?> config, String property) {
- return getBooleanConfig(config, property, false);
- }
-
static boolean getBooleanConfig(NamedList<?> config, String property,
boolean def) {
String envProp = EnvUtils.getProperty(property);
if (envProp == null) {
diff --git
a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3OutputStream.java
b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3OutputStream.java
index 25bf3465e7f..3f46905fa49 100644
--- a/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3OutputStream.java
+++ b/solr/modules/s3-repository/src/java/org/apache/solr/s3/S3OutputStream.java
@@ -151,7 +151,7 @@ public class S3OutputStream extends OutputStream {
}
// Flush is possible only if we have more data than the required part size
- // If buffer size is lower than than, just skip
+ // If buffer size is lower, then just skip
if (buffer.position() - buffer.arrayOffset() >= MIN_PART_SIZE) {
uploadPart();
}
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 6124d98e812..bf076226685 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
@@ -230,7 +230,7 @@ public class S3StorageClient {
}
/**
- * Delete directory, all the files and sub-directories from S3.
+ * Delete directory, all the files and subdirectories from S3.
*
* @param path Path to directory in S3.
*/
@@ -247,10 +247,10 @@ public class S3StorageClient {
}
/**
- * List all the files and sub-directories directly under given path.
+ * List all the files and subdirectories directly under given path.
*
* @param path Path to directory in S3.
- * @return Files and sub-directories in path.
+ * @return Files and subdirectories in path.
*/
String[] listDir(String path) throws S3Exception {
path = sanitizedDirPath(path);
@@ -432,7 +432,7 @@ public class S3StorageClient {
* Per the S3 docs:
*
https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/model/DeleteObjectsResult.html
* An exception is thrown if there's a client error processing the
request or in S3 itself.
- * However, there's no guarantee the delete did not happen if an
exception is thrown.
+ * However, there's no guarantee the delete operation did not happen if
an exception is thrown.
*/
return deleteObjects(paths, MAX_KEYS_PER_BATCH_DELETE);
} catch (SdkException sdke) {
@@ -548,7 +548,7 @@ public class S3StorageClient {
}
/** Ensures path adheres to some rules: -Doesn't start with a leading slash
*/
- String sanitizedPath(String path) throws S3Exception {
+ String sanitizedPath(String path) {
// Trim space from start and end
String sanitizedPath = path.trim();
@@ -584,7 +584,7 @@ public class S3StorageClient {
* Ensures directory path adheres to some rules: -Overall Path rules from
`sanitizedPath` -Add a
* trailing slash if one does not exist
*/
- String sanitizedDirPath(String path) throws S3Exception {
+ String sanitizedDirPath(String path) {
// Trim space from start and end
String sanitizedPath = sanitizedPath(path);
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 5d4f6c3af54..bd24ac0a7cc 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
@@ -120,18 +120,18 @@ public class S3BackupRepositoryTest extends
AbstractBackupRepositoryTest {
repo.createDirectory(path);
assertTrue(repo.exists(path));
assertEquals(BackupRepository.PathType.DIRECTORY,
repo.getPathType(path));
- assertEquals("No files should exist in dir yet",
repo.listAll(path).length, 0);
+ assertEquals("No files should exist in dir yet", 0,
repo.listAll(path).length);
URI subDir = new URI("/test/dir/");
repo.createDirectory(subDir);
assertTrue(repo.exists(subDir));
assertEquals(BackupRepository.PathType.DIRECTORY,
repo.getPathType(subDir));
- assertEquals("No files should exist in subdir yet",
repo.listAll(subDir).length, 0);
+ assertEquals("No files should exist in subdir yet", 0,
repo.listAll(subDir).length);
assertEquals(
"subDir should now be returned when listing all in parent dir",
- repo.listAll(path).length,
- 1);
+ 1,
+ repo.listAll(path).length);
repo.deleteDirectory(path);
assertFalse(repo.exists(path));
@@ -258,7 +258,7 @@ public class S3BackupRepositoryTest extends
AbstractBackupRepositoryTest {
* Check implementation of {@link S3BackupRepository#openInput(URI, String,
IOContext)}. Open an
* index input and seek to an absolute position.
*
- * <p>We use specified text. It must has the word "content" at given
position.
+ * <p>We use specified text. It must have the word "content" at given
position.
*/
private void doRandomAccessTest(String content, int position) throws
Exception {
@@ -304,7 +304,7 @@ public class S3BackupRepositoryTest extends
AbstractBackupRepositoryTest {
input.readBytes(buffer, 0, BufferedIndexInput.BUFFER_SIZE * 2);
// Seek back to the 5th byte.
- // It is not any more in the internal buffer, so we should fail
+ // It is not anymore in the internal buffer, so we should fail
IOException exception = assertThrows(IOException.class, () ->
input.seek(5));
assertEquals("Cannot seek backward", exception.getMessage());
}
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 f20d3f89cac..9546012190b 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
@@ -19,7 +19,6 @@ package org.apache.solr.s3;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
-import java.lang.invoke.MethodHandles;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.lucene.tests.util.QuickPatchThreadsFilter;
import org.apache.solr.SolrIgnoredThreadsFilter;
@@ -27,8 +26,6 @@ import
org.apache.solr.cloud.api.collections.AbstractIncrementalBackupTest;
import org.apache.solr.util.LogLevel;
import org.junit.BeforeClass;
import org.junit.ClassRule;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import software.amazon.awssdk.regions.Region;
// Backups do checksum validation against a footer value not present in
'SimpleText'
@@ -45,7 +42,6 @@ import software.amazon.awssdk.regions.Region;
value =
"org.apache.solr.cloud=DEBUG;org.apache.solr.cloud.api.collections=DEBUG;org.apache.solr.cloud.overseer=DEBUG")
public class S3IncrementalBackupTest extends AbstractIncrementalBackupTest {
- private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private static final String BUCKET_NAME =
S3IncrementalBackupTest.class.getSimpleName();
@@ -93,8 +89,6 @@ public class S3IncrementalBackupTest extends
AbstractIncrementalBackupTest {
+ " \n"
+ "</solr>\n";
- private static String backupLocation;
-
@BeforeClass
public static void ensureCompatibleLocale() {
// TODO: Find incompatible locales
diff --git
a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3OutputStreamTest.java
b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3OutputStreamTest.java
index 4df12150956..df1ae9991eb 100644
---
a/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3OutputStreamTest.java
+++
b/solr/modules/s3-repository/src/test/org/apache/solr/s3/S3OutputStreamTest.java
@@ -103,7 +103,7 @@ public class S3OutputStreamTest extends SolrTestCaseJ4 {
}
}
- /** Write a byte array larger than S3 part size. Simulate a real multi-part
upload. */
+ /** Write a byte array larger than S3 part size. Simulate a real multipart
upload. */
@Test
public void testWriteLargeBuffer() throws IOException {
// must be larger than S3 part size
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 9f360c644ac..3574587b4fa 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
@@ -99,7 +99,7 @@ public class S3ReadWriteTest extends AbstractS3ClientTest {
assertThat(exception.getMessage(), exception.getMessage(),
containsString("Path is Directory"));
}
- /** Check various method throws the expected exception of a missing S3 key.
*/
+ /** Check various methods throw the expected exception for a missing S3 key.
*/
@Test
public void testNotFound() {
assertThrows(S3NotFoundException.class, () ->
client.pullStream("/not-found"));
@@ -166,7 +166,7 @@ public class S3ReadWriteTest extends AbstractS3ClientTest {
break;
}
// Initiate a connection loss at the beginning of every
"bytesPerException" cycle.
- // The input stream will not immediately see an error, it will have
pre-loaded some data.
+ // The input stream will not immediately see an error, it will have
preloaded some data.
if ((byteCount % bytesPerException <= maxBuffer)) {
initiateS3ConnectionLoss();
}
