You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by ak...@apache.org on 2017/02/08 20:18:32 UTC

sentry git commit: SENTRY-1604: Sentry JSON message factory: Need more information in alter partition event (Nachiket Vaidya, Review by: Hao Hao, Vamsee Yarlagadda and Alex Kolbasov)

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 <ak...@cloudera.com>
Authored: Wed Feb 8 12:18:10 2017 -0800
Committer: Alexander Kolbasov <ak...@cloudera.com>
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());
     }
   }
 }