You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by sp...@apache.org on 2017/11/14 17:11:08 UTC

sentry git commit: Revert "SENTRY-2039: KeyValue is case sensitive and it causes incompatibility issues with external components (Sergio Pena, reviewed by Alexander Kolbasov, Na Li)"

Repository: sentry
Updated Branches:
  refs/heads/master a001d4399 -> 151ba5358


Revert "SENTRY-2039: KeyValue is case sensitive and it causes incompatibility issues with external components (Sergio Pena, reviewed by Alexander Kolbasov, Na Li)"

The case-sensitive issue is solved by each of the external components by using a specific model and using the CommonPrivilege interface. My old tests were based
on a different Privilege interace (such asIndexerWildcardPrivilege), so my tests were failing, but I learned tha kafka,solr,hive,sqoop uses the same CommonPrivilege
and they work.

This reverts commit a001d43992eab3d543927580da1f7533a3279897.


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/151ba535
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/151ba535
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/151ba535

Branch: refs/heads/master
Commit: 151ba5358697fd600c20f9fab8ea053b12d95bd9
Parents: a001d43
Author: Sergio Pena <se...@cloudera.com>
Authored: Tue Nov 14 11:08:36 2017 -0600
Committer: Sergio Pena <se...@cloudera.com>
Committed: Tue Nov 14 11:08:36 2017 -0600

----------------------------------------------------------------------
 .../apache/sentry/core/common/utils/KeyValue.java    | 15 ++-------------
 .../sentry/core/common/utils/TestKeyValue.java       |  7 -------
 .../sentry/policy/common/TestCommonPrivilege.java    | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/151ba535/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
index 500b98f..4e944e5 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
@@ -68,17 +68,6 @@ public class KeyValue {
     return result;
   }
 
-  /**
-   * Compares the specified object with this KeyValue for equality.
-   *
-   * Returns true if and only if the specified object is also a KeyValue and the corresponding
-   * key-value pair in the two KeyValue objects are equals. Key and value strings are compared
-   * for equality as case-insensitive (Two KeyValue objects are equal if (k1.equalsIgnoreCase(k2)
-   * && v1.equalsIgnoreCase(v1))).
-   *
-   * @param obj the object to be compared for equality with this KeyValue
-   * @return true if the specified object is equal to this KeyValue
-   */
   @Override
   public boolean equals(Object obj) {
     if (this == obj) {
@@ -95,14 +84,14 @@ public class KeyValue {
       if (other.key != null) {
         return false;
       }
-    } else if (!key.equalsIgnoreCase(other.key)) {
+    } else if (!key.equals(other.key)) {
       return false;
     }
     if (value == null) {
       if (other.value != null) {
         return false;
       }
-    } else if (!value.equalsIgnoreCase(other.value)) {
+    } else if (!value.equals(other.value)) {
       return false;
     }
     return true;

http://git-wip-us.apache.org/repos/asf/sentry/blob/151ba535/sentry-core/sentry-core-common/src/test/java/org/apache/sentry/core/common/utils/TestKeyValue.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/test/java/org/apache/sentry/core/common/utils/TestKeyValue.java b/sentry-core/sentry-core-common/src/test/java/org/apache/sentry/core/common/utils/TestKeyValue.java
index 0c5dfcc..ca44a24 100644
--- a/sentry-core/sentry-core-common/src/test/java/org/apache/sentry/core/common/utils/TestKeyValue.java
+++ b/sentry-core/sentry-core-common/src/test/java/org/apache/sentry/core/common/utils/TestKeyValue.java
@@ -55,13 +55,6 @@ public class TestKeyValue {
     doTest(kv1, kv2, kv3);
   }
 
-  @Test
-  public void testKeyValueCaseInsensitive() throws Exception {
-    KeyValue kv1 = new KeyValue("k1", "v1");
-    KeyValue kv2 = new KeyValue("K1", "V1");
-    Assert.assertEquals(kv1, kv2);
-  }
-
   private void doTest(KeyValue kv1, KeyValue kv2, KeyValue kv3) {
     Assert.assertEquals(kv1, kv2);
     Assert.assertFalse(kv1.equals(kv3));

http://git-wip-us.apache.org/repos/asf/sentry/blob/151ba535/sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestCommonPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestCommonPrivilege.java b/sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestCommonPrivilege.java
index abaf61f..3f60b19 100644
--- a/sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestCommonPrivilege.java
+++ b/sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestCommonPrivilege.java
@@ -129,4 +129,19 @@ public class TestCommonPrivilege {
     assertFalse(privilegForSelect.implies(privilegForAll, testModel));
     assertFalse(privilegForInsert.implies(privilegForAll, testModel));
   }
+
+  @Test
+  public void testImplyStringCaseSensitive() throws Exception {
+    CommonPrivilege privileg1 = new CommonPrivilege("server=server1->db=db1->table=table1->column=col1->action=select");
+    CommonPrivilege privileg2 = new CommonPrivilege("server=server1->db=db1->table=table1->column=CoL1->action=select");
+    CommonPrivilege privileg3 = new CommonPrivilege("server=SERver1->db=Db1->table=TAbLe1->column=col1->action=select");
+    CommonPrivilege privileg4 = new CommonPrivilege("SERVER=server1->DB=db1->TABLE=table1->COLUMN=col1->ACTION=select");
+
+    // column is case sensitive
+    assertFalse(privileg1.implies(privileg2, testModel));
+    // server, db, table is case insensitive
+    assertTrue(privileg1.implies(privileg3, testModel));
+    // key in privilege is case insensitive
+    assertTrue(privileg1.implies(privileg4, testModel));
+  }
 }