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

krathbun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
     new 0cf01e3365 Fix broken NamespacesIT, table ops exceptions changes 
(#5715)
0cf01e3365 is described below

commit 0cf01e3365b2493e84c9a3e243a26bf8acec80d4
Author: Kevin Rathbun <krath...@apache.org>
AuthorDate: Fri Jul 25 09:36:12 2025 -0400

    Fix broken NamespacesIT, table ops exceptions changes (#5715)
    
    * Fix broken IT: NamespacesIT
    
    * TServerClient.execute() and executeVoid() exception changes:
    
    Previously:
    * Thrown to caller: ThriftTableOperationException wrapped in an 
AccumuloException
    Now:
    * If the ThriftTableOperationException is the result of the table or 
namespace not existing, throw to caller: TableNotFoundException wrapped in an 
AccumuloException
    * If the ThriftTableOperationException occurs for any other reason, throw 
to caller: ThriftTableOperationException wrapped in an AccumuloException
    
    Callers in TableOperationsImpl, NamespaceOperationsImpl, and 
SecurityOperationsImpl:
    * would previously handle the ThriftTableOperationException wrapped in an 
AccumuloException, throwing a more appropriate exception to the user (e.g., 
TableNotFoundException, NamespaceNotFoundException, AccumuloSecurityException) 
if the cause was the table or namespace not existing. Would throw the 
ThriftTableOperationException wrapped in an AccumuloException otherwise.
    * now handle the TableNotFoundException wrapped in an AccumuloException, 
throwing a more appropriate exception to the user (e.g., 
TableNotFoundException, NamespaceNotFoundException, AccumuloSecurityException) 
if needed. Throws the ThriftTableOperationException wrapped in an 
AccumuloException if it is not a TableNotFoundException.
    
    Effect of this change on InstanceOperationsImpl:
    * Callers in InstanceOperationsImpl call execute and executeVoid directly, 
throwing the exception as-is to the user. So, this does change functionality 
here as previously a user would see an AccumuloException which wraps a 
ThriftTableOperationException for any cause of the TTOE (table doesn't exist, 
namespace doesn't exist, other). Now, users will see an AccumuloException which 
wraps a TableNotFoundException if the cause was due to the namespace or table 
not existing; will see an Accu [...]
---
 .../core/clientImpl/NamespaceOperationsImpl.java   | 45 +++++++++---------
 .../core/clientImpl/SecurityOperationsImpl.java    | 55 +++++++++++++---------
 .../core/clientImpl/TableOperationsImpl.java       | 50 +++++++-------------
 .../accumulo/core/rpc/clients/TServerClient.java   | 29 ++++++++++++
 .../accumulo/test/NamespacesIT_SimpleSuite.java    |  1 +
 5 files changed, 105 insertions(+), 75 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
index 34094cd2c2..1792208687 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java
@@ -49,8 +49,6 @@ import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.client.admin.TableOperations;
 import org.apache.accumulo.core.clientImpl.thrift.SecurityErrorCode;
 import org.apache.accumulo.core.clientImpl.thrift.TVersionedProperties;
-import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType;
-import 
org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
 import org.apache.accumulo.core.data.NamespaceId;
 import org.apache.accumulo.core.data.constraints.Constraint;
 import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
@@ -299,13 +297,14 @@ public class NamespaceOperationsImpl extends 
NamespaceOperationsHelper {
     } catch (AccumuloSecurityException e) {
       throw e;
     } catch (AccumuloException e) {
-      Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) {
-          throw new NamespaceNotFoundException(ttoe);
+      Throwable eCause = e.getCause();
+      if (eCause instanceof TableNotFoundException) {
+        Throwable tnfeCause = eCause.getCause();
+        if (tnfeCause instanceof NamespaceNotFoundException) {
+          var nnfe = (NamespaceNotFoundException) tnfeCause;
+          nnfe.addSuppressed(e);
+          throw nnfe;
         }
-        throw e;
       }
       throw e;
     } catch (Exception e) {
@@ -322,13 +321,14 @@ public class NamespaceOperationsImpl extends 
NamespaceOperationsHelper {
       return ThriftClientTypes.CLIENT.execute(context, client -> client
           .getNamespaceProperties(TraceUtil.traceInfo(), context.rpcCreds(), 
namespace));
     } catch (AccumuloException e) {
-      Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) {
-          throw new NamespaceNotFoundException(ttoe);
+      Throwable eCause = e.getCause();
+      if (eCause instanceof TableNotFoundException) {
+        Throwable tnfeCause = eCause.getCause();
+        if (tnfeCause instanceof NamespaceNotFoundException) {
+          var nnfe = (NamespaceNotFoundException) tnfeCause;
+          nnfe.addSuppressed(e);
+          throw nnfe;
         }
-        throw e;
       }
       throw e;
     } catch (Exception e) {
@@ -356,14 +356,17 @@ public class NamespaceOperationsImpl extends 
NamespaceOperationsHelper {
       return ThriftClientTypes.CLIENT.execute(context,
           client -> client.checkNamespaceClass(TraceUtil.traceInfo(), 
context.rpcCreds(), namespace,
               className, asTypeName));
-    } catch (AccumuloSecurityException | AccumuloException e) {
-      Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) {
-          throw new NamespaceNotFoundException(ttoe);
+    } catch (AccumuloSecurityException e) {
+      throw e;
+    } catch (AccumuloException e) {
+      Throwable eCause = e.getCause();
+      if (eCause instanceof TableNotFoundException) {
+        Throwable tnfeCause = eCause.getCause();
+        if (tnfeCause instanceof NamespaceNotFoundException) {
+          var nnfe = (NamespaceNotFoundException) tnfeCause;
+          nnfe.addSuppressed(e);
+          throw nnfe;
         }
-        throw e;
       }
       throw e;
     } catch (Exception e) {
diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java
index 23114926ef..d074b1544e 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java
@@ -27,6 +27,7 @@ import java.util.Set;
 
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
 import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.client.admin.DelegationTokenConfig;
 import org.apache.accumulo.core.client.admin.SecurityOperations;
@@ -64,17 +65,22 @@ public class SecurityOperationsImpl implements 
SecurityOperations {
       throws AccumuloException, AccumuloSecurityException {
     try {
       ThriftClientTypes.CLIENT.executeVoid(context, client -> 
exec.execute(client));
-    } catch (AccumuloSecurityException | AccumuloException e) {
-      Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        // recast missing table
-        if (ttoe.getType() == TableOperationExceptionType.NOTFOUND) {
-          throw new AccumuloSecurityException(null, 
SecurityErrorCode.TABLE_DOESNT_EXIST);
-        } else if (ttoe.getType() == 
TableOperationExceptionType.NAMESPACE_NOTFOUND) {
-          throw new AccumuloSecurityException(null, 
SecurityErrorCode.NAMESPACE_DOESNT_EXIST);
-        } else {
-          throw e;
+    } catch (AccumuloSecurityException e) {
+      throw e;
+    } catch (AccumuloException e) {
+      Throwable eCause = e.getCause();
+      if (eCause instanceof TableNotFoundException) {
+        var tnfeCause = eCause.getCause();
+        if (tnfeCause instanceof NamespaceNotFoundException) {
+          var ase = new AccumuloSecurityException(null, 
SecurityErrorCode.NAMESPACE_DOESNT_EXIST);
+          ase.addSuppressed(e);
+          throw ase;
+        } else if (tnfeCause instanceof ThriftTableOperationException
+            && ((ThriftTableOperationException) tnfeCause).getType()
+                == TableOperationExceptionType.NOTFOUND) {
+          var ase = new AccumuloSecurityException(null, 
SecurityErrorCode.TABLE_DOESNT_EXIST);
+          ase.addSuppressed(e);
+          throw ase;
         }
       }
       throw e;
@@ -96,17 +102,22 @@ public class SecurityOperationsImpl implements 
SecurityOperations {
       throws AccumuloException, AccumuloSecurityException {
     try {
       return ThriftClientTypes.CLIENT.execute(context, client -> 
exec.execute(client));
-    } catch (AccumuloSecurityException | AccumuloException e) {
-      Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        // recast missing table
-        if (ttoe.getType() == TableOperationExceptionType.NOTFOUND) {
-          throw new AccumuloSecurityException(null, 
SecurityErrorCode.TABLE_DOESNT_EXIST);
-        } else if (ttoe.getType() == 
TableOperationExceptionType.NAMESPACE_NOTFOUND) {
-          throw new AccumuloSecurityException(null, 
SecurityErrorCode.NAMESPACE_DOESNT_EXIST);
-        } else {
-          throw e;
+    } catch (AccumuloSecurityException e) {
+      throw e;
+    } catch (AccumuloException e) {
+      Throwable eCause = e.getCause();
+      if (eCause instanceof TableNotFoundException) {
+        var tnfeCause = eCause.getCause();
+        if (tnfeCause instanceof NamespaceNotFoundException) {
+          var ase = new AccumuloSecurityException(null, 
SecurityErrorCode.NAMESPACE_DOESNT_EXIST);
+          ase.addSuppressed(e);
+          throw ase;
+        } else if (tnfeCause instanceof ThriftTableOperationException
+            && ((ThriftTableOperationException) tnfeCause).getType()
+                == TableOperationExceptionType.NOTFOUND) {
+          var ase = new AccumuloSecurityException(null, 
SecurityErrorCode.TABLE_DOESNT_EXIST);
+          ase.addSuppressed(e);
+          throw ase;
         }
       }
       throw e;
diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
index ba8bba53f1..356fb6d129 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
@@ -1123,7 +1123,9 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
     } catch (AccumuloException ae) {
       Throwable cause = ae.getCause();
       if (cause instanceof TableNotFoundException) {
-        throw new TableNotFoundException(null, tableName, null, ae);
+        var tnfe = (TableNotFoundException) cause;
+        tnfe.addSuppressed(ae);
+        throw tnfe;
       }
       throw ae;
     }
@@ -1181,16 +1183,10 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
           .getTableConfiguration(TraceUtil.traceInfo(), context.rpcCreds(), 
tableName));
     } catch (AccumuloException e) {
       Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        switch (ttoe.getType()) {
-          case NOTFOUND:
-            throw new TableNotFoundException(ttoe);
-          case NAMESPACE_NOTFOUND:
-            throw new TableNotFoundException(tableName, new 
NamespaceNotFoundException(ttoe));
-          default:
-            throw e;
-        }
+      if (t instanceof TableNotFoundException) {
+        var tnfe = (TableNotFoundException) t;
+        tnfe.addSuppressed(e);
+        throw tnfe;
       }
       throw e;
     } catch (Exception e) {
@@ -1208,16 +1204,10 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
           .getTableProperties(TraceUtil.traceInfo(), context.rpcCreds(), 
tableName));
     } catch (AccumuloException e) {
       Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        switch (ttoe.getType()) {
-          case NOTFOUND:
-            throw new TableNotFoundException(ttoe);
-          case NAMESPACE_NOTFOUND:
-            throw new TableNotFoundException(tableName, new 
NamespaceNotFoundException(ttoe));
-          default:
-            throw e;
-        }
+      if (t instanceof TableNotFoundException) {
+        var tnfe = (TableNotFoundException) t;
+        tnfe.addSuppressed(e);
+        throw tnfe;
       }
       throw e;
     } catch (Exception e) {
@@ -1798,18 +1788,14 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
       return ThriftClientTypes.CLIENT.execute(context,
           client -> client.checkTableClass(TraceUtil.traceInfo(), 
context.rpcCreds(), tableName,
               className, asTypeName));
-    } catch (AccumuloSecurityException | AccumuloException e) {
+    } catch (AccumuloSecurityException e) {
+      throw e;
+    } catch (AccumuloException e) {
       Throwable t = e.getCause();
-      if (t instanceof ThriftTableOperationException) {
-        ThriftTableOperationException ttoe = (ThriftTableOperationException) t;
-        switch (ttoe.getType()) {
-          case NOTFOUND:
-            throw new TableNotFoundException(ttoe);
-          case NAMESPACE_NOTFOUND:
-            throw new TableNotFoundException(tableName, new 
NamespaceNotFoundException(ttoe));
-          default:
-            throw e;
-        }
+      if (t instanceof TableNotFoundException) {
+        var tnfe = (TableNotFoundException) t;
+        tnfe.addSuppressed(e);
+        throw tnfe;
       }
       throw e;
     } catch (Exception e) {
diff --git 
a/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java 
b/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java
index e84a7beb34..3539b853e6 100644
--- a/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java
+++ b/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java
@@ -33,9 +33,12 @@ import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.clientImpl.AccumuloServerException;
 import org.apache.accumulo.core.clientImpl.ClientContext;
 import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import 
org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
 import org.apache.accumulo.core.lock.ServiceLockData;
 import org.apache.accumulo.core.lock.ServiceLockData.ThriftService;
 import org.apache.accumulo.core.lock.ServiceLockPaths.AddressSelector;
@@ -174,6 +177,19 @@ public interface TServerClient<C extends TServiceClient> {
       } catch (TTransportException tte) {
         LOG.debug("ClientService request failed " + server + ", retrying ... 
", tte);
         sleepUninterruptibly(100, MILLISECONDS);
+      } catch (ThriftTableOperationException ttoe) {
+        TableNotFoundException tnfe;
+        switch (ttoe.getType()) {
+          case NOTFOUND:
+            tnfe = new TableNotFoundException(ttoe);
+            throw new AccumuloException(tnfe);
+          case NAMESPACE_NOTFOUND:
+            tnfe = new TableNotFoundException(ttoe.getTableName(),
+                new NamespaceNotFoundException(ttoe));
+            throw new AccumuloException(tnfe);
+          default:
+            throw new AccumuloException(ttoe);
+        }
       } catch (TException e) {
         throw new AccumuloException(e);
       } finally {
@@ -202,6 +218,19 @@ public interface TServerClient<C extends TServiceClient> {
       } catch (TTransportException tte) {
         LOG.debug("ClientService request failed " + server + ", retrying ... 
", tte);
         sleepUninterruptibly(100, MILLISECONDS);
+      } catch (ThriftTableOperationException ttoe) {
+        TableNotFoundException tnfe;
+        switch (ttoe.getType()) {
+          case NOTFOUND:
+            tnfe = new TableNotFoundException(ttoe);
+            throw new AccumuloException(tnfe);
+          case NAMESPACE_NOTFOUND:
+            tnfe = new TableNotFoundException(ttoe.getTableName(),
+                new NamespaceNotFoundException(ttoe));
+            throw new AccumuloException(tnfe);
+          default:
+            throw new AccumuloException(ttoe);
+        }
       } catch (TException e) {
         throw new AccumuloException(e);
       } finally {
diff --git 
a/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java 
b/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java
index c04a47cde6..4134d30fe7 100644
--- a/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java
+++ b/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java
@@ -1160,6 +1160,7 @@ public class NamespacesIT_SimpleSuite extends 
SharedMiniClusterBase {
     assertNoNamespace(() -> ops.setProperty(namespace, "k", "v"));
     assertNoNamespace(() -> ops.testClassLoad(namespace, 
VersioningIterator.class.getName(),
         SortedKeyValueIterator.class.getName()));
+    assertNoNamespace(() -> ops.getNamespaceProperties(namespace));
 
     // namespace operations that should throw a NamespaceExistsException
     assertNamespaceExists(() -> ops.create(Namespace.DEFAULT.name()));

Reply via email to