Fix NullPointer

ExpediaGroup · May 6, 2024 · e7b3e03 · e7b3e03
1 parent 5ad57a3
commit e7b3e03
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,8 @@
 
+## [3.13.1] - 2024-05-06
+### Fix
+- Fix for `NullPointerException` in ClientCapabilities when `get_table_req` and `get_tables_req` is called from Hive3.x client. See [#317](https://github.com/ExpediaGroup/waggle-dance/issues/317)
+
 ## [3.13.0] - 2024-04-19
 ### Added
 - Added `waggle-dance-extensions` module. See [extensions README](waggle-dance-extensions/README.md.)

diff --git a/...ance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java b/...ance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java
@@ -16,6 +16,7 @@
 package com.hotels.bdp.waggledance.mapping.model;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.hadoop.hive.metastore.MetaStoreFilterHook;
@@ -24,6 +25,8 @@
 import org.apache.hadoop.hive.metastore.api.AddPartitionsResult;
 import org.apache.hadoop.hive.metastore.api.AlreadyExistsException;
 import org.apache.hadoop.hive.metastore.api.CacheFileMetadataRequest;
+import org.apache.hadoop.hive.metastore.api.ClientCapabilities;
+import org.apache.hadoop.hive.metastore.api.ClientCapability;
 import org.apache.hadoop.hive.metastore.api.ColumnStatistics;
 import org.apache.hadoop.hive.metastore.api.CompactionRequest;
 import org.apache.hadoop.hive.metastore.api.Database;
@@ -166,7 +169,7 @@ public Function transformOutboundFunction(Function function) {
   @Override
   public HiveObjectRef transformInboundHiveObjectRef(HiveObjectRef obj) {
     obj.setDbName(metaStoreMapping.transformInboundDatabaseName(obj.getDbName()));
-    if (obj.getObjectName()!=null && obj.getObjectType() == HiveObjectType.DATABASE) {
+    if (obj.getObjectName() != null && obj.getObjectType() == HiveObjectType.DATABASE) {
       obj.setObjectName(metaStoreMapping.transformInboundDatabaseName(obj.getObjectName()));
     }
     return obj;
@@ -480,9 +483,22 @@ public List<PartitionSpec> transformInboundPartitionSpecs(List<PartitionSpec> pa
   @Override
   public GetTableRequest transformInboundGetTableRequest(GetTableRequest request) {
     request.setDbName(metaStoreMapping.transformInboundDatabaseName(request.getDbName()));
+    cleanupClientCapabilities(request.getCapabilities());
     return request;
   }
 
+  private void cleanupClientCapabilities(ClientCapabilities clientCapabilities) {
+    if (clientCapabilities != null) {
+      List<ClientCapability> values = new ArrayList<>();
+      for (ClientCapability value : clientCapabilities.getValues()) {
+        if (value != null) {
+          values.add(value);
+        }
+      }
+      clientCapabilities.setValues(values);
+    }
+  }
+
   @Override
   public GetTableResult transformOutboundGetTableResult(GetTableResult result) {
     transformOutboundTable(result.getTable());
@@ -492,6 +508,7 @@ public GetTableResult transformOutboundGetTableResult(GetTableResult result) {
   @Override
   public GetTablesRequest transformInboundGetTablesRequest(GetTablesRequest request) {
     request.setDbName(metaStoreMapping.transformInboundDatabaseName(request.getDbName()));
+    cleanupClientCapabilities(request.getCapabilities());
     return request;
   }
 

diff --git a/...-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java b/...-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java
@@ -21,6 +21,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -34,6 +35,8 @@
 import org.apache.hadoop.hive.metastore.api.AddPartitionsRequest;
 import org.apache.hadoop.hive.metastore.api.AddPartitionsResult;
 import org.apache.hadoop.hive.metastore.api.CacheFileMetadataRequest;
+import org.apache.hadoop.hive.metastore.api.ClientCapabilities;
+import org.apache.hadoop.hive.metastore.api.ClientCapability;
 import org.apache.hadoop.hive.metastore.api.ColumnStatistics;
 import org.apache.hadoop.hive.metastore.api.ColumnStatisticsDesc;
 import org.apache.hadoop.hive.metastore.api.CompactionRequest;
@@ -72,14 +75,14 @@
 import org.apache.hadoop.hive.metastore.api.Table;
 import org.apache.hadoop.hive.metastore.api.TableMeta;
 import org.apache.hadoop.hive.metastore.api.TableStatsRequest;
+import org.apache.thrift.TException;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import com.google.common.collect.Lists;
-
 import com.hotels.bdp.waggledance.api.WaggleDanceException;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -759,8 +762,56 @@ public void transformInboundGetTableRequest() throws Exception {
     assertThat(transformedRequest, is(sameInstance(request)));
     assertThat(transformedRequest.getDbName(), is(IN_DB_NAME));
     assertThat(transformedRequest.getTblName(), is(TABLE_NAME));
+    try {
+      transformedRequest.validate();
+    } catch (TException e) {
+      fail("Validation should not fail");
+    }
+  }
+
+  @Test
+  public void transformInboundGetTableRequestClientCapabilities() throws Exception {
+    GetTableRequest request = new GetTableRequest();
+    request.setDbName(DB_NAME);
+    request.setTblName(TABLE_NAME);
+    ClientCapabilities clientCapabilities = new ClientCapabilities();
+    clientCapabilities.setValues(Lists.newArrayList(ClientCapability.TEST_CAPABILITY, null));
+    request.setCapabilities(clientCapabilities);
+    GetTableRequest transformedRequest = databaseMapping.transformInboundGetTableRequest(request);
+    assertThat(transformedRequest, is(sameInstance(request)));
+    assertThat(transformedRequest.getDbName(), is(IN_DB_NAME));
+    assertThat(transformedRequest.getTblName(), is(TABLE_NAME));
+    assertThat(transformedRequest.getCapabilities().getValues().size(), is(1));
+    assertThat(transformedRequest.getCapabilities().getValues().get(0), is(ClientCapability.TEST_CAPABILITY));
+    try {
+      transformedRequest.validate();
+    } catch (TException e) {
+      fail("Validation should not fail");
+    }
+
+  }
+
+  @Test
+  public void transformInboundGetTableRequestClientCapabilitiesIsNull() throws Exception {
+    GetTableRequest request = new GetTableRequest();
+    request.setDbName(DB_NAME);
+    request.setTblName(TABLE_NAME);
+    ClientCapabilities clientCapabilities = new ClientCapabilities();
+    clientCapabilities.setValues(Lists.newArrayList((ClientCapability)null));
+    request.setCapabilities(clientCapabilities);
+    GetTableRequest transformedRequest = databaseMapping.transformInboundGetTableRequest(request);
+    assertThat(transformedRequest, is(sameInstance(request)));
+    assertThat(transformedRequest.getDbName(), is(IN_DB_NAME));
+    assertThat(transformedRequest.getTblName(), is(TABLE_NAME));
+    assertThat(transformedRequest.getCapabilities().getValues().size(), is(1));
+    try {
+      transformedRequest.validate();
+    } catch (TException e) {
+      fail("Validation should not fail");
+    }
   }
 
+
   @Test
   public void transformOutboundGetTableResult() throws Exception {
     Table table = new Table();
@@ -786,6 +837,55 @@ public void transformInboundGetTablesRequest() throws Exception {
     assertThat(transformedRequest, is(sameInstance(request)));
     assertThat(transformedRequest.getDbName(), is(IN_DB_NAME));
     assertThat(transformedRequest.getTblNames(), is(Collections.singletonList(TABLE_NAME)));
+    try {
+      transformedRequest.validate();
+    } catch (TException e) {
+      fail("Validation should not fail");
+    }
+
+  }
+
+  @Test
+  public void transformInboundGetTablesRequestClientCapabilities() throws Exception {
+    GetTablesRequest request = new GetTablesRequest();
+    request.setDbName(DB_NAME);
+    request.setTblNames(Collections.singletonList(TABLE_NAME));
+    ClientCapabilities clientCapabilities = new ClientCapabilities();
+    clientCapabilities.setValues(Lists.newArrayList(ClientCapability.TEST_CAPABILITY, null));
+    request.setCapabilities(clientCapabilities);
+
+    GetTablesRequest transformedRequest = databaseMapping.transformInboundGetTablesRequest(request);
+    assertThat(transformedRequest, is(sameInstance(request)));
+    assertThat(transformedRequest.getDbName(), is(IN_DB_NAME));
+    assertThat(transformedRequest.getTblNames(), is(Collections.singletonList(TABLE_NAME)));
+    assertThat(transformedRequest.getCapabilities().getValues().size(), is(1));
+    assertThat(transformedRequest.getCapabilities().getValues().get(0), is(ClientCapability.TEST_CAPABILITY));
+    try {
+      transformedRequest.validate();
+    } catch (TException e) {
+      fail("Validation should not fail");
+    }
+  }
+
+  @Test
+  public void transformInboundGetTablesRequestClientCapabilitiesIsNull() throws Exception {
+    GetTablesRequest request = new GetTablesRequest();
+    request.setDbName(DB_NAME);
+    request.setTblNames(Collections.singletonList(TABLE_NAME));
+    ClientCapabilities clientCapabilities = new ClientCapabilities();
+    clientCapabilities.setValues(Lists.newArrayList((ClientCapability)null));
+    request.setCapabilities(clientCapabilities);
+
+    GetTablesRequest transformedRequest = databaseMapping.transformInboundGetTablesRequest(request);
+    assertThat(transformedRequest, is(sameInstance(request)));
+    assertThat(transformedRequest.getDbName(), is(IN_DB_NAME));
+    assertThat(transformedRequest.getTblNames(), is(Collections.singletonList(TABLE_NAME)));
+    assertThat(transformedRequest.getCapabilities().getValues().size(), is(0));
+    try {
+      transformedRequest.validate();
+    } catch (TException e) {
+      fail("Validation should not fail");
+    }
   }
 
   @Test