From 7bea2d50133c6b7000462ee6b8c1d78ae6d57d40 Mon Sep 17 00:00:00 2001 From: "Bala.FA" Date: Sat, 17 Oct 2020 12:56:01 +0530 Subject: [PATCH] Refactor iterator in removeObjects() API. Fixes #1075 --- api/src/main/java/io/minio/MinioClient.java | 72 +++++++------------ .../main/java/io/minio/RemoveObjectsArgs.java | 10 --- functional/FunctionalTest.java | 52 ++++++++++---- 3 files changed, 63 insertions(+), 71 deletions(-) diff --git a/api/src/main/java/io/minio/MinioClient.java b/api/src/main/java/io/minio/MinioClient.java index cc3ddbede..136e19c25 100644 --- a/api/src/main/java/io/minio/MinioClient.java +++ b/api/src/main/java/io/minio/MinioClient.java @@ -1721,13 +1721,16 @@ public Iterable> removeObjects(RemoveObjectsArgs args) { @Override public Iterator> iterator() { return new Iterator>() { - private Result error; - private Iterator errorIterator; + private Result error = null; + private Iterator errorIterator = null; private boolean completed = false; private Iterator objectIter = args.objects().iterator(); private synchronized void populate() { - List errorList = null; + if (completed) { + return; + } + try { List objectList = new LinkedList<>(); int i = 0; @@ -1736,17 +1739,21 @@ private synchronized void populate() { i++; } - if (objectList.size() > 0) { + completed = objectList.size() == 0; + if (!completed) { DeleteObjectsResponse response = deleteObjects( args.bucket(), args.region(), objectList, - args.quiet(), + true, args.bypassGovernanceMode(), args.extraHeaders(), args.extraQueryParams()); - errorList = response.result().errorList(); + if (response.result().errorList().size() > 0) { + errorIterator = response.result().errorList().iterator(); + completed = true; + } } } catch (ErrorResponseException | InsufficientDataException @@ -1757,66 +1764,35 @@ private synchronized void populate() { | NoSuchAlgorithmException | ServerException | XmlParserException e) { - this.error = new Result<>(e); - } finally { - if (errorList != null) { - this.errorIterator = errorList.iterator(); - } else { - this.errorIterator = new LinkedList().iterator(); - } + error = new Result<>(e); + completed = true; } } @Override public boolean hasNext() { - if (this.completed) { - return false; - } - - if (this.error == null && this.errorIterator == null) { + while (error == null && errorIterator == null && !completed) { populate(); } - if (this.error == null && this.errorIterator != null && !this.errorIterator.hasNext()) { - populate(); - } - - if (this.error != null) { - return true; - } - - if (this.errorIterator.hasNext()) { + if (error != null || (errorIterator != null && errorIterator.hasNext())) { return true; } - this.completed = true; - return false; + return !completed; } @Override public Result next() { - if (this.completed) { - throw new NoSuchElementException(); - } - - if (this.error == null && this.errorIterator == null) { - populate(); - } - - if (this.error == null && this.errorIterator != null && !this.errorIterator.hasNext()) { - populate(); - } - - if (this.error != null) { - this.completed = true; - return this.error; - } + if (!hasNext()) throw new NoSuchElementException(); - if (this.errorIterator.hasNext()) { - return new Result<>(this.errorIterator.next()); + if (error != null) { + return error; + } else if (errorIterator != null && errorIterator.hasNext()) { + return new Result<>(errorIterator.next()); } - this.completed = true; + // This never happens. throw new NoSuchElementException(); } diff --git a/api/src/main/java/io/minio/RemoveObjectsArgs.java b/api/src/main/java/io/minio/RemoveObjectsArgs.java index 6b8c1797f..ba9d04791 100644 --- a/api/src/main/java/io/minio/RemoveObjectsArgs.java +++ b/api/src/main/java/io/minio/RemoveObjectsArgs.java @@ -23,7 +23,6 @@ public class RemoveObjectsArgs extends BucketArgs { private boolean bypassGovernanceMode; private Iterable objects = new LinkedList<>(); - private boolean quiet; public boolean bypassGovernanceMode() { return bypassGovernanceMode; @@ -33,10 +32,6 @@ public Iterable objects() { return objects; } - public boolean quiet() { - return quiet; - } - public static Builder builder() { return new Builder(); } @@ -53,10 +48,5 @@ public Builder objects(Iterable objects) { operations.add(args -> args.objects = objects); return this; } - - public Builder quiet(boolean flag) { - operations.add(args -> args.quiet = flag); - return this; - } } } diff --git a/functional/FunctionalTest.java b/functional/FunctionalTest.java index 58252b848..55ba12e8e 100644 --- a/functional/FunctionalTest.java +++ b/functional/FunctionalTest.java @@ -1519,28 +1519,54 @@ public static void removeObject() throws Exception { RemoveObjectArgs.builder().bucket(bucketName).object(getRandomName()).build()); } + public static void testRemoveObjects(String testTags, List results) + throws Exception { + String methodName = "removeObjects()"; + long startTime = System.currentTimeMillis(); + try { + removeObjects(bucketName, results); + mintSuccessLog(methodName, testTags, startTime); + } catch (Exception e) { + handleException(methodName, testTags, startTime, e); + } finally { + removeObjects(bucketName, results); + } + } + public static void removeObjects() throws Exception { String methodName = "removeObjects()"; if (!mintEnv) { System.out.println(methodName); } + testRemoveObjects("[basic]", createObjects(bucketName, 3, 0)); + + String testTags = "[3005 objects]"; long startTime = System.currentTimeMillis(); + String objectName = getRandomName(); + List results = new LinkedList<>(); + for (int i = 0; i < 3004; i++) { + results.add( + new ObjectWriteResponse(null, bucketName, null, objectName + "-" + i, null, null)); + } + List existingObject = createObjects(bucketName, 1, 0); + results.addAll(existingObject); + testRemoveObjects(testTags, results); try { - List results = null; - try { - results = createObjects(bucketName, 3, 0); - results.add( - new ObjectWriteResponse(null, bucketName, null, "nonexistent-object", null, null)); - removeObjects(bucketName, results); - mintSuccessLog(methodName, null, startTime); - } finally { - if (results != null) { - removeObjects(bucketName, results); - } + client.statObject( + StatObjectArgs.builder() + .bucket(bucketName) + .object(existingObject.get(0).object()) + .build()); + handleException( + methodName, + testTags, + startTime, + new Exception("object " + existingObject.get(0).object() + " still exist")); + } catch (ErrorResponseException e) { + if (!e.errorResponse().code().equals("NoSuchKey")) { + throw e; } - } catch (Exception e) { - handleException(methodName, null, startTime, e); } }