From e7b3e03d337531b758fd76efdea54644ec044701 Mon Sep 17 00:00:00 2001 From: paduin Date: Mon, 6 May 2024 11:50:44 +0200 Subject: [PATCH] Fix NullPointer --- CHANGELOG.md | 4 + .../mapping/model/DatabaseMappingImpl.java | 19 +++- .../model/DatabaseMappingImplTest.java | 102 +++++++++++++++++- 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a59ad4dbf..800155cf0 100644 --- 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/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java index b30ea8e7f..8654c2983 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java +++ b/waggle-dance-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 transformInboundPartitionSpecs(List 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 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/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java index 1f14e7a9a..c644f21f1 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java +++ b/waggle-dance-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,6 +75,7 @@ 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; @@ -79,7 +83,6 @@ 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