fix: properly validate that aspects can only be of type record (#144)

* properly validate that aspects can only be of type record

* add more tests to validation util

* remove union test

* change name of method

dao-api/src/main/pegasus/com/linkedin/metadata/dummy/DummyAspect.pdl

@@ -1,8 +1,6 @@
 namespace com.linkedin.metadata.dummy
 
-import com.linkedin.common.Urn
-
 /**
  * Not to be used
  */
-typeref DummyAspect = union[Urn]
+typeref DummyAspect = union[DummyRecord]

dao-api/src/main/pegasus/com/linkedin/metadata/dummy/DummyRecord.pdl

@@ -0,0 +1,9 @@
+namespace com.linkedin.metadata.dummy
+
+/**
+ * Not to be used
+ */
+record DummyRecord {
+
+  value: string
+}

testing/test-models/src/main/pegasus/com/linkedin/testing/PizzaArrayAspect.pdl

@@ -0,0 +1,5 @@
+namespace com.linkedin.testing
+
+typeref PizzaArrayAspect = union[
+  array[long]
+]

testing/test-models/src/main/pegasus/com/linkedin/testing/PizzaEnumAspect.pdl

@@ -0,0 +1,5 @@
+namespace com.linkedin.testing
+
+typeref PizzaEnumAspect = union[
+  PizzaSize
+]

testing/test-models/src/main/pegasus/com/linkedin/testing/PizzaMapAspect.pdl

@@ -0,0 +1,5 @@
+namespace com.linkedin.testing
+
+typeref PizzaMapAspect = union[
+  map[string, boolean]
+]

testing/test-models/src/main/pegasus/com/linkedin/testing/PizzaStringAspect.pdl

@@ -0,0 +1,5 @@
+namespace com.linkedin.testing
+
+typeref PizzaStringAspect = union[
+  string
+]

validators/build.gradle

@@ -12,4 +12,5 @@ dependencies {
   annotationProcessor externalDependency.lombok
 
   testCompile externalDependency.guava
+  testCompile project(':testing:test-models')
 }

validators/src/main/java/com/linkedin/metadata/validator/AspectValidator.java

@@ -28,7 +28,7 @@ private AspectValidator() {
    */
   public static void validateAspectUnionSchema(@Nonnull UnionDataSchema schema, @Nonnull String aspectClassName) {
 
-    if (!ValidationUtils.isUnionWithOnlyComplexMembers(schema)) {
+    if (!ValidationUtils.isUnionWithOnlyRecordMembers(schema)) {
       ValidationUtils.invalidSchema("Aspect '%s' must be a union containing only record type members", aspectClassName);
     }
   }
@@ -47,7 +47,7 @@ public static void validateAspectUnionSchema(@Nonnull Class<? extends UnionTempl
 
   private static boolean isValidMetadataField(RecordDataSchema.Field field) {
     return field.getName().equals("metadata") && !field.getOptional()
-        && field.getType().getType() == DataSchema.Type.UNION && ValidationUtils.isUnionWithOnlyComplexMembers(
+        && field.getType().getType() == DataSchema.Type.UNION && ValidationUtils.isUnionWithOnlyRecordMembers(
         (UnionDataSchema) field.getType());
   }
 }

validators/src/main/java/com/linkedin/metadata/validator/EntityValidator.java

@@ -89,7 +89,7 @@ public static void validateEntityUnionSchema(@Nonnull Class<? extends UnionTempl
    */
   public static void validateEntityUnionSchema(@Nonnull UnionDataSchema schema, @Nonnull String entityClassName) {
 
-    if (!ValidationUtils.isUnionWithOnlyComplexMembers(schema)) {
+    if (!ValidationUtils.isUnionWithOnlyRecordMembers(schema)) {
       ValidationUtils.invalidSchema("Entity '%s' must be a union containing only record type members", entityClassName);
     }
   }

validators/src/main/java/com/linkedin/metadata/validator/RelationshipValidator.java

@@ -94,7 +94,7 @@ public static void validateRelationshipUnionSchema(@Nonnull Class<? extends Unio
    */
   public static void validateRelationshipUnionSchema(@Nonnull UnionDataSchema schema, @Nonnull String relationshipClassName) {
 
-    if (!ValidationUtils.isUnionWithOnlyComplexMembers(schema)) {
+    if (!ValidationUtils.isUnionWithOnlyRecordMembers(schema)) {
       ValidationUtils.invalidSchema("Relationship '%s' must be a union containing only record type members", relationshipClassName);
     }
   }

validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java

@@ -157,8 +157,9 @@ public static List<RecordDataSchema.Field> fieldsUsingInvalidType(@Nonnull Recor
         .collect(Collectors.toList());
   }
 
-  public static boolean isUnionWithOnlyComplexMembers(UnionDataSchema unionDataSchema) {
-    return unionDataSchema.getMembers().stream().allMatch(member -> member.getType().isComplex());
+  public static boolean isUnionWithOnlyRecordMembers(UnionDataSchema unionDataSchema) {
+    return unionDataSchema.getMembers().stream().allMatch(member -> member.getType().getDereferencedDataSchema().getType().equals(
+        DataSchema.Type.RECORD));
   }
 
   @Nonnull

validators/src/test/java/com/linkedin/metadata/validator/ValidationUtilsTest.java

@@ -0,0 +1,38 @@
+package com.linkedin.metadata.validator;
+
+import com.linkedin.data.schema.UnionDataSchema;
+import com.linkedin.testing.PizzaArrayAspect;
+import com.linkedin.testing.PizzaEnumAspect;
+import com.linkedin.testing.PizzaMapAspect;
+import com.linkedin.testing.PizzaStringAspect;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.*;
+
+
+public class ValidationUtilsTest {
+
+  @Test
+  public void testUnionWithEnums() {
+    UnionDataSchema dataSchema = ValidationUtils.getUnionSchema(PizzaEnumAspect.class);
+    assertFalse(ValidationUtils.isUnionWithOnlyRecordMembers(dataSchema));
+  }
+
+  @Test
+  public void testUnionWithMaps() {
+    UnionDataSchema dataSchema = ValidationUtils.getUnionSchema(PizzaMapAspect.class);
+    assertFalse(ValidationUtils.isUnionWithOnlyRecordMembers(dataSchema));
+  }
+
+  @Test
+  public void testUnionWithArrays() {
+    UnionDataSchema dataSchema = ValidationUtils.getUnionSchema(PizzaArrayAspect.class);
+    assertFalse(ValidationUtils.isUnionWithOnlyRecordMembers(dataSchema));
+  }
+
+  @Test
+  public void testUnionWithPrimitive() {
+    UnionDataSchema dataSchema = ValidationUtils.getUnionSchema(PizzaStringAspect.class);
+    assertFalse(ValidationUtils.isUnionWithOnlyRecordMembers(dataSchema));
+  }
+}