Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign e840bdb15 -> a79f923dc
SENTRY-1604: Sentry JSON message factory: Need more information in alter partition event (Nachiket Vaidya, Review by: Hao Hao, Vamsee Yarlagadda and Alex Kolbasov) Currently, message factory for alter partition does not store new partition values. It just stores old values. For alter partition with partition change, we need both values. In this fix: - sentry message factory stores also new partition values in alter partition message in addition to old values. - Added unit test cases for the same. - Added new field "newValues" to store new partition values in SentryJSONAlterPartitionMessage. Testing done: Added unit test cases. Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/a79f923d Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/a79f923d Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/a79f923d Branch: refs/heads/sentry-ha-redesign Commit: a79f923dc61d050ddf88dafc87399c9b00577fc2 Parents: e840bdb Author: Alexander Kolbasov <[email protected]> Authored: Wed Feb 8 12:18:10 2017 -0800 Committer: Alexander Kolbasov <[email protected]> Committed: Wed Feb 8 12:18:10 2017 -0800 ---------------------------------------------------------------------- .../json/SentryJSONAlterPartitionMessage.java | 9 +++++++- .../json/SentryJSONMessageFactory.java | 2 +- ...actMetastoreTestWithStaticConfiguration.java | 5 +++++ ...NotificationListenerInBuiltDeserializer.java | 23 +++++++++++++++++++- .../TestSentryListenerSentryDeserializer.java | 17 +++++++++++++++ 5 files changed, 53 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/a79f923d/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java b/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java index 89ee863..161bf4d 100644 --- a/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java +++ b/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java @@ -29,6 +29,8 @@ public class SentryJSONAlterPartitionMessage extends JSONAlterPartitionMessage { private String newLocation; @JsonProperty private String oldLocation; + @JsonProperty + private List<String> newValues; public SentryJSONAlterPartitionMessage() { super("", "", "", "", ImmutableList.<String>of(), null); @@ -36,12 +38,13 @@ public class SentryJSONAlterPartitionMessage extends JSONAlterPartitionMessage { public SentryJSONAlterPartitionMessage(String server, String servicePrincipal, String db, String table, - List<String> values, + List<String> values, List<String> newValues, Long timestamp, String oldlocation, String newLocation) { super(server, servicePrincipal, db, table, values, timestamp); this.newLocation = newLocation; this.oldLocation = oldlocation; + this.newValues = newValues; } public String getNewLocation() { @@ -52,6 +55,10 @@ public class SentryJSONAlterPartitionMessage extends JSONAlterPartitionMessage { return oldLocation; } + public List<String> getNewValues() { + return newValues; + } + @Override public String toString() { return SentryJSONMessageDeserializer.serialize(this); http://git-wip-us.apache.org/repos/asf/sentry/blob/a79f923d/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java b/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java index 1fc11f8..b949ee5 100644 --- a/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java +++ b/sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java @@ -107,7 +107,7 @@ public class SentryJSONMessageFactory extends MessageFactory { @Override public SentryJSONAlterPartitionMessage buildAlterPartitionMessage(Partition before, Partition after) { return new SentryJSONAlterPartitionMessage(HCAT_SERVER_URL, HCAT_SERVICE_PRINCIPAL, before.getDbName(), - before.getTableName(), before.getValues(), now(), before.getSd().getLocation(), + before.getTableName(), before.getValues(), after.getValues(), now(), before.getSd().getLocation(), after.getSd().getLocation()); } http://git-wip-us.apache.org/repos/asf/sentry/blob/a79f923d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java index 18720eb..5f6ad0f 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java @@ -132,6 +132,11 @@ public abstract class AbstractMetastoreTestWithStaticConfiguration extends client.alter_partition(partition.getDbName(), partition.getTableName(), partition); } + public void renamePartition(HiveMetaStoreClient client, Partition partition, Partition newPartition) throws Exception { + client.renamePartition(partition.getDbName(), partition.getTableName(), partition.getValues(), + newPartition); + } + public void dropPartition(HiveMetaStoreClient client, String dbName, String tblName, List<String> ptnVals) throws Exception { client.dropPartition(dbName, tblName, ptnVals); http://git-wip-us.apache.org/repos/asf/sentry/blob/a79f923d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java index 56e19c4..e9b3a43 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java @@ -32,6 +32,7 @@ import org.apache.hive.hcatalog.messaging.AlterPartitionMessage; import org.apache.hive.hcatalog.messaging.DropDatabaseMessage; import org.apache.hive.hcatalog.messaging.AddPartitionMessage; import org.apache.hive.hcatalog.messaging.DropPartitionMessage; +import org.apache.sentry.binding.metastore.messaging.json.SentryJSONAlterPartitionMessage; import org.apache.sentry.tests.e2e.hive.StaticUserGroup; import org.apache.sentry.tests.e2e.hive.hiveserver.HiveServerFactory; import org.hamcrest.text.IsEqualIgnoringCase; @@ -347,7 +348,27 @@ public class TestDBNotificationListenerInBuiltDeserializer extends AbstractMetas assertEquals(HCatEventMessage.EventType.ALTER_PARTITION, alterPartitionMessage.getEventType()); assertThat(alterPartitionMessage.getDB(), IsEqualIgnoringCase.equalToIgnoringCase(testDB));// dbName assertThat(alterPartitionMessage.getTable(), IsEqualIgnoringCase.equalToIgnoringCase(testTable));// tableName - //Location information, not sure how can we get this? Refresh all paths for every alter table add partition? + assertEquals(partVals1, alterPartitionMessage.getValues()); + if (alterPartitionMessage instanceof SentryJSONAlterPartitionMessage) { + SentryJSONAlterPartitionMessage sjAlterPartitionMessage = (SentryJSONAlterPartitionMessage) alterPartitionMessage; + assertEquals(partVals1, sjAlterPartitionMessage.getNewValues()); + } + + Partition newPartition = partition.deepCopy(); + ArrayList<String> partVals2 = Lists.newArrayList("part2"); + newPartition.setValues(partVals2); + renamePartition(client, partition, newPartition); + latestID = client.getCurrentNotificationEventId(); + response = client.getNextNotification(latestID.getEventId()-1, 1, null); + alterPartitionMessage = deserializer.getAlterPartitionMessage(response.getEvents().get(0).getMessage()); + assertEquals(HCatEventMessage.EventType.ALTER_PARTITION, alterPartitionMessage.getEventType()); + assertThat(alterPartitionMessage.getDB(), IsEqualIgnoringCase.equalToIgnoringCase(testDB));// dbName + assertThat(alterPartitionMessage.getTable(), IsEqualIgnoringCase.equalToIgnoringCase(testTable));// tableName + assertEquals(partVals1, alterPartitionMessage.getValues()); + if (alterPartitionMessage instanceof SentryJSONAlterPartitionMessage) { + SentryJSONAlterPartitionMessage sjAlterPartitionMessage = (SentryJSONAlterPartitionMessage) alterPartitionMessage; + assertEquals(partVals2, sjAlterPartitionMessage.getNewValues()); + } } } http://git-wip-us.apache.org/repos/asf/sentry/blob/a79f923d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java index 5a51d93..fe72b91 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java @@ -374,6 +374,23 @@ public class TestSentryListenerSentryDeserializer extends AbstractMetastoreTestW if(!useDbNotificationListener) { Assert.assertEquals(oldLocation.toLowerCase(), alterPartitionMessage.getOldLocation()); Assert.assertEquals(newLocation.toLowerCase(), alterPartitionMessage.getNewLocation()); + assertEquals(partVals1, alterPartitionMessage.getValues()); + assertEquals(partVals1, alterPartitionMessage.getNewValues()); + } + + Partition newPartition = partition.deepCopy(); + ArrayList<String> partVals2 = Lists.newArrayList("part2"); + newPartition.setValues(partVals2); + renamePartition(client, partition, newPartition); + latestID = client.getCurrentNotificationEventId(); + response = client.getNextNotification(latestID.getEventId() - 1, 1, null); + alterPartitionMessage = deserializer.getAlterPartitionMessage(response.getEvents().get(0).getMessage()); + assertEquals(HCatEventMessage.EventType.ALTER_PARTITION, alterPartitionMessage.getEventType()); + assertThat(alterPartitionMessage.getDB(), IsEqualIgnoringCase.equalToIgnoringCase(testDB));// dbName + assertThat(alterPartitionMessage.getTable(), IsEqualIgnoringCase.equalToIgnoringCase(testTable));// tableName + if(!useDbNotificationListener) { + assertEquals(partVals1, alterPartitionMessage.getValues()); + assertEquals(partVals2, alterPartitionMessage.getNewValues()); } } }
