update logic and expectations

datahub-project · Nov 27, 2024 · 3b3c1b8 · 3b3c1b8
1 parent 4b31103
commit 3b3c1b8
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 23 deletions.
diff --git a/docs/what/urn.md b/docs/what/urn.md
@@ -39,14 +39,13 @@ There are a few restrictions when creating an URN:
 
 The following characters are not allowed anywhere in the URN
 
-1. Commas are reserved character in URN fields: `,`
-2. Parentheses are reserved characters in URN fields: `(` or `)`
-3. URN separator UTF-8 character `␟`
+1. Parentheses are reserved characters in URN fields: `(` or `)`
+2. URN separator UTF-8 character `␟`
 
-The following characters are allowed within an URN tuple only.
+The following characters are not allowed within an URN tuple.
 
-1. Colons are reserved characters in URN fields: `:`
+1. Commas are reserved characters in URN tuples: `,`
 
-Example: `urn:li:dashboard:(looker,dashboards.thelook::cohort_data_tool)`
+Example: Adding another `,` between the parentheses in this URN `urn:li:dashboard:(looker,dashboards.thelook)` is invalid.
 
 Please do not use these characters when creating or generating urns. One approach is to use URL encoding for the characters.
diff --git a/...data-io-api/src/main/java/com/linkedin/metadata/entity/validation/ValidationApiUtils.java b/...data-io-api/src/main/java/com/linkedin/metadata/entity/validation/ValidationApiUtils.java
@@ -30,8 +30,8 @@ public class ValidationApiUtils {
   // Related to BrowsePathv2
   public static final String URN_DELIMITER_SEPARATOR = "␟";
   // https://datahubproject.io/docs/what/urn/#restrictions
-  public static final Set<String> ILLEGAL_URN_COMPONENT_CHARACTERS = Set.of(":", "(", ")", ",");
-  public static final String URN_TUPLE_ALLOWED_CHARACTERS_REGEX = "[:]";
+  public static final Set<String> ILLEGAL_URN_COMPONENT_CHARACTERS = Set.of("(", ")");
+  public static final Set<String> ILLEGAL_URN_TUPLE_CHARACTERS = Set.of(",");
 
   /**
    * Validates a {@link RecordTemplate} and throws {@link ValidationException} if validation fails.
@@ -91,8 +91,6 @@ public static void validateUrn(
     List<String> illegalComponents =
         urn.getEntityKey().getParts().stream()
             .flatMap(part -> processUrnPartRecursively(part, totalParts))
-            .filter(
-                urnPart -> ILLEGAL_URN_COMPONENT_CHARACTERS.stream().anyMatch(urnPart::contains))
             .collect(Collectors.toList());
 
     if (!illegalComponents.isEmpty()) {
@@ -126,9 +124,15 @@ private static Stream<String> processUrnPartRecursively(String urnPart, int tota
           .flatMap(part -> processUrnPartRecursively(part, nestedParts));
     }
     if (totalParts > 1) {
-      return Stream.of(urnPart.replaceAll(URN_TUPLE_ALLOWED_CHARACTERS_REGEX, "%3A"));
+      if (ILLEGAL_URN_TUPLE_CHARACTERS.stream().anyMatch(c -> urnPart.contains(c))) {
+        return Stream.of(urnPart);
+      }
+    }
+    if (ILLEGAL_URN_COMPONENT_CHARACTERS.stream().anyMatch(c -> urnPart.contains(c))) {
+      return Stream.of(urnPart);
     }
-    return Stream.of(urnPart);
+
+    return Stream.empty();
   }
 
   /**

diff --git a/...-io-api/src/test/java/com/linkedin/metadata/entity/validation/ValidationApiUtilsTest.java b/...-io-api/src/test/java/com/linkedin/metadata/entity/validation/ValidationApiUtilsTest.java
@@ -18,10 +18,36 @@ public void testValidateDatasetUrn() {
     // If no exception is thrown, test passes
   }
 
-  @Test(expectedExceptions = IllegalArgumentException.class)
+  @Test
   public void testSimpleUrnColon() {
-    Urn invalidUrn = UrnUtils.getUrn("urn:li:corpuser:foo:bar");
-    ValidationApiUtils.validateUrn(entityRegistry, invalidUrn, true);
+    ValidationApiUtils.validateUrn(
+        entityRegistry, UrnUtils.getUrn("urn:li:corpuser:foo:bar"), true);
+    ValidationApiUtils.validateUrn(
+        entityRegistry, UrnUtils.getUrn("urn:li:dataPlatform:abc:def"), true);
+    ValidationApiUtils.validateUrn(
+        entityRegistry, UrnUtils.getUrn("urn:li:corpuser:foo:[email protected]"), true);
+    // If no exception is thrown, test passes
+  }
+
+  @Test
+  public void testSimpleUrnComma() {
+    ValidationApiUtils.validateUrn(entityRegistry, UrnUtils.getUrn("urn:li:corpuser:,"), true);
+    // If no exception is thrown, test passes
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  public void testTupleUrnComma() {
+    ValidationApiUtils.validateUrn(
+        entityRegistry, UrnUtils.getUrn("urn:li:dashboard:(looker,dashboards,thelook)"), true);
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  public void testFabricTypeCasing() {
+    // prod != PROD
+    ValidationApiUtils.validateUrn(
+        entityRegistry,
+        UrnUtils.getUrn("urn:li:dataset:(urn:li:dataPlatform:abc:def,table_name,prod)"),
+        true);
   }
 
   @Test
@@ -34,7 +60,7 @@ public void testComplexUrnColon() throws URISyntaxException {
   }
 
   @Test(expectedExceptions = IllegalArgumentException.class)
-  public void testUrnFabricType() {
+  public void testFabricTypeParen() {
     Urn invalidUrn = UrnUtils.getUrn("urn:li:dataset:(urn:li:dataPlatform:hdfs,/path/to/data,())");
     ValidationApiUtils.validateUrn(entityRegistry, invalidUrn, true);
   }
@@ -119,15 +145,10 @@ public void testValidColon() {
     // If no exception is thrown, test passes
   }
 
-  @Test(expectedExceptions = IllegalArgumentException.class)
-  public void testNoTupleColon() {
-    Urn invalidUrn = UrnUtils.getUrn("urn:li:corpuser::");
-    ValidationApiUtils.validateUrn(entityRegistry, invalidUrn, true);
-  }
-
-  @Test(expectedExceptions = IllegalArgumentException.class)
+  @Test
   public void testNoTupleComma() {
     Urn invalidUrn = UrnUtils.getUrn("urn:li:corpuser:,");
     ValidationApiUtils.validateUrn(entityRegistry, invalidUrn, true);
+    // If no exception is thrown, test passes
   }
 }