athrog commented on a change in pull request #271:
URL: https://github.com/apache/solr/pull/271#discussion_r695971805
##########
File path:
solr/contrib/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
##########
@@ -194,55 +204,48 @@ void deleteDirectory(String path) throws S3Exception {
String[] listDir(String path) throws S3Exception {
path = sanitizedDirPath(path);
- String prefix = path;
- ListObjectsRequest listRequest =
- new ListObjectsRequest()
- .withBucketName(bucketName)
- .withPrefix(prefix)
- .withDelimiter(S3_FILE_PATH_DELIMITER);
+ final String prefix = path;
List<String> entries = new ArrayList<>();
try {
- ObjectListing objectListing = s3Client.listObjects(listRequest);
-
- while (true) {
- List<String> files =
- objectListing.getObjectSummaries().stream()
- .map(S3ObjectSummary::getKey)
- .collect(Collectors.toList());
- files.addAll(objectListing.getCommonPrefixes());
- // This filtering is needed only for S3mock. Real S3 does not ignore
the trailing '/' in the
- // prefix.
- files =
- files.stream()
- .filter(s -> s.startsWith(prefix))
- .map(s -> s.substring(prefix.length()))
- .filter(s -> !s.isEmpty())
- .filter(
- s -> {
- int slashIndex = s.indexOf(S3_FILE_PATH_DELIMITER);
- return slashIndex == -1 || slashIndex == s.length() - 1;
- })
- .map(
- s -> {
- if (s.endsWith(S3_FILE_PATH_DELIMITER)) {
- return s.substring(0, s.length() - 1);
- }
- return s;
- })
- .collect(Collectors.toList());
-
- entries.addAll(files);
-
- if (objectListing.isTruncated()) {
- objectListing = s3Client.listNextBatchOfObjects(objectListing);
- } else {
- break;
- }
- }
+ ListObjectsV2Iterable objectListing =
+ s3Client.listObjectsV2Paginator(
+ builder ->
+ builder
+ .bucket(bucketName)
+ .prefix(prefix)
+ .delimiter(S3_FILE_PATH_DELIMITER)
+ .build());
+
+ List<String> files =
+
objectListing.contents().stream().map(S3Object::key).collect(Collectors.toList());
+
objectListing.commonPrefixes().stream().map(CommonPrefix::prefix).forEach(files::add);
+ // This filtering is needed only for S3mock. Real S3 does not ignore the
trailing '/' in the
+ // prefix.
+ files =
Review comment:
Now that the `while` loop is gone and we can get everything in one API
call, this streaming/filtering between `files`, `objectListing`, and `entries`
(and even the returned `String[]`) could be simplified.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
