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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org