[bookkeeper] branch branch-4.8 updated: Open ledger returns no ledger exception if ledger id is negative

[sijie]

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch branch-4.8
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/branch-4.8 by this push:
     new be41178  Open ledger returns no ledger exception if ledger id is 
negative
be41178 is described below

commit be41178599fa2ab367841f6f139fd1c6c696bfc5
Author: Qi Wang <codingwan...@gmail.com>
AuthorDate: Thu Sep 6 14:14:13 2018 -0700

    Open ledger returns no ledger exception if ledger id is negative
    
    Descriptions of the changes in this PR:
    
    ### Motivation
    
    if I open a ledger with negative value, bookkeeper client responds 
'NoSuchLedgerException'. But I think it should return `IllegalOpException`, 
because it is an illegal exception.
    
    Author: Qi Wang <codingwan...@gmail.com>
    
    Reviewers: Enrico Olivelli <eolive...@gmail.com>, Sijie Guo 
<si...@apache.org>
    
    This closes #1638 from codingwangqi/fix_return_code
    
    (cherry picked from commit 40185e1f39fafd0c796cec38d463ebd55edaa250)
    Signed-off-by: Sijie Guo <si...@apache.org>
---
 .../src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java   | 6 ++++--
 .../java/org/apache/bookkeeper/client/impl/OpenBuilderBase.java    | 7 ++++---
 .../src/test/java/org/apache/bookkeeper/client/MockBookKeeper.java | 6 ++++--
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java
index c3a69bd..5580c06 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java
@@ -33,6 +33,7 @@ import 
org.apache.bookkeeper.client.AsyncCallback.OpenCallback;
 import org.apache.bookkeeper.client.AsyncCallback.ReadLastConfirmedCallback;
 import org.apache.bookkeeper.client.BookKeeper.DigestType;
 import org.apache.bookkeeper.client.SyncCallbackUtils.SyncOpenCallback;
+import org.apache.bookkeeper.client.api.BKException.Code;
 import org.apache.bookkeeper.client.api.ReadHandle;
 import org.apache.bookkeeper.client.impl.OpenBuilderBase;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
@@ -242,8 +243,9 @@ class LedgerOpenOp implements 
GenericCallback<LedgerMetadata> {
         }
 
         private void open(OpenCallback cb) {
-            if (!validate()) {
-                cb.openComplete(BKException.Code.NoSuchLedgerExistsException, 
null, null);
+            final int validateRc = validate();
+            if (Code.OK != validateRc) {
+                cb.openComplete(validateRc, null, null);
                 return;
             }
 
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/impl/OpenBuilderBase.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/impl/OpenBuilderBase.java
index b22effc..c2c4c35 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/impl/OpenBuilderBase.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/impl/OpenBuilderBase.java
@@ -21,6 +21,7 @@ package org.apache.bookkeeper.client.impl;
 import java.util.Arrays;
 
 import org.apache.bookkeeper.client.LedgerHandle;
+import org.apache.bookkeeper.client.api.BKException.Code;
 import org.apache.bookkeeper.client.api.DigestType;
 import org.apache.bookkeeper.client.api.OpenBuilder;
 
@@ -62,11 +63,11 @@ public abstract class OpenBuilderBase implements 
OpenBuilder {
         return this;
     }
 
-    protected boolean validate() {
+    protected int validate() {
         if (ledgerId < 0) {
             LOG.error("invalid ledgerId {} < 0", ledgerId);
-            return false;
+            return Code.NoSuchLedgerExistsException;
         }
-        return true;
+        return Code.OK;
     }
 }
diff --git 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeper.java
 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeper.java
index 3b63cfd..c65e070 100644
--- 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeper.java
+++ 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeper.java
@@ -35,6 +35,7 @@ import java.util.concurrent.atomic.AtomicLong;
 import org.apache.bookkeeper.client.AsyncCallback.CreateCallback;
 import org.apache.bookkeeper.client.AsyncCallback.DeleteCallback;
 import org.apache.bookkeeper.client.AsyncCallback.OpenCallback;
+import org.apache.bookkeeper.client.api.BKException.Code;
 import org.apache.bookkeeper.client.api.OpenBuilder;
 import org.apache.bookkeeper.client.api.ReadHandle;
 import org.apache.bookkeeper.client.impl.OpenBuilderBase;
@@ -221,8 +222,9 @@ public class MockBookKeeper extends BookKeeper {
             public CompletableFuture<ReadHandle> execute() {
                 CompletableFuture<ReadHandle> promise = new 
CompletableFuture<ReadHandle>();
 
-                if (!validate()) {
-                    promise.completeExceptionally(new 
BKException.BKNoSuchLedgerExistsException());
+                final int validateRc = validate();
+                if (Code.OK != validateRc) {
+                    
promise.completeExceptionally(BKException.create(validateRc));
                     return promise;
                 } else if (getProgrammedFailStatus()) {
                     if (failReturnCode != BkTimeoutOperation) {