mneethiraj commented on code in PR #490:
URL: https://github.com/apache/atlas/pull/490#discussion_r2652365399


##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -363,44 +369,91 @@ private AtlasBusinessMetadataDef 
toBusinessMetadataDef(AtlasVertex vertex) throw
     private void checkBusinessMetadataRef(String typeName) throws 
AtlasBaseException {
         AtlasBusinessMetadataDef businessMetadataDef = 
typeRegistry.getBusinessMetadataDefByName(typeName);
 
-        if (businessMetadataDef != null) {
-            List<AtlasStructDef.AtlasAttributeDef> attributeDefs = 
businessMetadataDef.getAttributeDefs();
+        if (businessMetadataDef == null || 
CollectionUtils.isEmpty(businessMetadataDef.getAttributeDefs())) {
+            return;
+        }
 
-            for (AtlasStructDef.AtlasAttributeDef attributeDef : 
attributeDefs) {
-                String      qualifiedName      = 
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(businessMetadataDef, 
attributeDef.getName());
-                String      vertexPropertyName = 
AtlasStructType.AtlasAttribute.generateVertexPropertyName(businessMetadataDef, 
attributeDef, qualifiedName);
-                Set<String> applicableTypes    = 
AtlasJson.fromJson(attributeDef.getOption(AtlasBusinessMetadataDef.ATTR_OPTION_APPLICABLE_ENTITY_TYPES),
 Set.class);
+        for (AtlasStructDef.AtlasAttributeDef attributeDef : 
businessMetadataDef.getAttributeDefs()) {
+            validateAttributeReferences(businessMetadataDef, attributeDef);
+        }
+    }
 
-                if (CollectionUtils.isNotEmpty(applicableTypes) && 
isBusinessAttributePresent(vertexPropertyName, applicableTypes)) {
-                    throw new 
AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName);
-                }
-            }
+    private void validateAttributeReferences(AtlasBusinessMetadataDef bmDef, 
AtlasStructDef.AtlasAttributeDef attributeDef) throws AtlasBaseException {
+        String applicableTypesStr = 
attributeDef.getOption(ATTR_OPTION_APPLICABLE_ENTITY_TYPES);
+        Set<String> applicableTypes = StringUtils.isBlank(applicableTypesStr) 
? null : AtlasJson.fromJson(applicableTypesStr, Set.class);
+
+        if (CollectionUtils.isEmpty(applicableTypes)) {
+            return;
+        }
+
+        String qualifiedName      = 
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(bmDef, 
attributeDef.getName());
+        String vertexPropertyName = 
AtlasStructType.AtlasAttribute.generateVertexPropertyName(bmDef, attributeDef, 
qualifiedName);
+
+        boolean isPresent;
+        long startTime = System.currentTimeMillis();
+
+        if (Boolean.TRUE.equals(attributeDef.getIsIndexable())) {
+            isPresent = isBusinessAttributePresent(vertexPropertyName, 
applicableTypes);
+        } else {
+            isPresent = isBusinessAttributePresentInGraph(vertexPropertyName, 
applicableTypes);
+        }
+
+        if (LOG.isDebugEnabled()) {
+            LOG.info("Reference check for attribute {} took {} ms. Found: {}",
+                    attributeDef.getName(), (System.currentTimeMillis() - 
startTime), isPresent);
+        }
+
+        if (isPresent) {
+            throw new AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, 
bmDef.getName());
         }
     }
 
     private boolean isBusinessAttributePresent(String attrName, Set<String> 
applicableTypes) throws AtlasBaseException {
         SearchParameters.FilterCriteria criteria = new 
SearchParameters.FilterCriteria();
-
         criteria.setAttributeName(attrName);
-        criteria.setOperator(SearchParameters.Operator.NOT_EMPTY);
+        criteria.setOperator(SearchParameters.Operator.NOT_NULL);
 
         SearchParameters.FilterCriteria entityFilters = new 
SearchParameters.FilterCriteria();
-
         
entityFilters.setCondition(SearchParameters.FilterCriteria.Condition.OR);
         entityFilters.setCriterion(Collections.singletonList(criteria));
 
         SearchParameters searchParameters = new SearchParameters();
-
         
searchParameters.setTypeName(String.join(SearchContext.TYPENAME_DELIMITER, 
applicableTypes));
         searchParameters.setExcludeDeletedEntities(true);
         searchParameters.setIncludeSubClassifications(false);
         searchParameters.setEntityFilters(entityFilters);
-        searchParameters.setAttributes(Collections.singleton(attrName));
-        searchParameters.setOffset(0);
+        searchParameters.setAttributes(Collections.emptySet());
         searchParameters.setLimit(1);
 
         AtlasSearchResult atlasSearchResult = 
entityDiscoveryService.searchWithParameters(searchParameters);
+        return atlasSearchResult != null && 
CollectionUtils.isNotEmpty(atlasSearchResult.getEntities());
+    }
+
+    private boolean isBusinessAttributePresentInGraph(String 
vertexPropertyName, Set<String> applicableTypes) {
+        if (graph == null || CollectionUtils.isEmpty(applicableTypes)) {
+            return false;
+        }
+
+        try {
+            for (String typeName : applicableTypes) {
 
-        return CollectionUtils.isNotEmpty(atlasSearchResult.getEntities());
+                Iterable<AtlasVertex> vertices = graph.query()
+                        .has(Constants.ENTITY_TYPE_PROPERTY_KEY, typeName)

Review Comment:
   Is it necessary to add `ENTITY_TYPE_PROPERTY_KEY` filter here? If yes, note 
that all subTypes of types in`applicableTypes` should be checked as well; 
consider a business attribute associated with a type like `DataSet`.



##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -363,44 +369,91 @@ private AtlasBusinessMetadataDef 
toBusinessMetadataDef(AtlasVertex vertex) throw
     private void checkBusinessMetadataRef(String typeName) throws 
AtlasBaseException {
         AtlasBusinessMetadataDef businessMetadataDef = 
typeRegistry.getBusinessMetadataDefByName(typeName);
 
-        if (businessMetadataDef != null) {
-            List<AtlasStructDef.AtlasAttributeDef> attributeDefs = 
businessMetadataDef.getAttributeDefs();
+        if (businessMetadataDef == null || 
CollectionUtils.isEmpty(businessMetadataDef.getAttributeDefs())) {
+            return;
+        }
 
-            for (AtlasStructDef.AtlasAttributeDef attributeDef : 
attributeDefs) {
-                String      qualifiedName      = 
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(businessMetadataDef, 
attributeDef.getName());
-                String      vertexPropertyName = 
AtlasStructType.AtlasAttribute.generateVertexPropertyName(businessMetadataDef, 
attributeDef, qualifiedName);
-                Set<String> applicableTypes    = 
AtlasJson.fromJson(attributeDef.getOption(AtlasBusinessMetadataDef.ATTR_OPTION_APPLICABLE_ENTITY_TYPES),
 Set.class);
+        for (AtlasStructDef.AtlasAttributeDef attributeDef : 
businessMetadataDef.getAttributeDefs()) {
+            validateAttributeReferences(businessMetadataDef, attributeDef);
+        }
+    }
 
-                if (CollectionUtils.isNotEmpty(applicableTypes) && 
isBusinessAttributePresent(vertexPropertyName, applicableTypes)) {
-                    throw new 
AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName);
-                }
-            }
+    private void validateAttributeReferences(AtlasBusinessMetadataDef bmDef, 
AtlasStructDef.AtlasAttributeDef attributeDef) throws AtlasBaseException {
+        String applicableTypesStr = 
attributeDef.getOption(ATTR_OPTION_APPLICABLE_ENTITY_TYPES);
+        Set<String> applicableTypes = StringUtils.isBlank(applicableTypesStr) 
? null : AtlasJson.fromJson(applicableTypesStr, Set.class);
+
+        if (CollectionUtils.isEmpty(applicableTypes)) {
+            return;
+        }
+
+        String qualifiedName      = 
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(bmDef, 
attributeDef.getName());
+        String vertexPropertyName = 
AtlasStructType.AtlasAttribute.generateVertexPropertyName(bmDef, attributeDef, 
qualifiedName);
+
+        boolean isPresent;
+        long startTime = System.currentTimeMillis();
+
+        if (Boolean.TRUE.equals(attributeDef.getIsIndexable())) {
+            isPresent = isBusinessAttributePresent(vertexPropertyName, 
applicableTypes);
+        } else {
+            isPresent = isBusinessAttributePresentInGraph(vertexPropertyName, 
applicableTypes);
+        }
+
+        if (LOG.isDebugEnabled()) {
+            LOG.info("Reference check for attribute {} took {} ms. Found: {}",
+                    attributeDef.getName(), (System.currentTimeMillis() - 
startTime), isPresent);
+        }
+
+        if (isPresent) {
+            throw new AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, 
bmDef.getName());
         }
     }
 
     private boolean isBusinessAttributePresent(String attrName, Set<String> 
applicableTypes) throws AtlasBaseException {
         SearchParameters.FilterCriteria criteria = new 
SearchParameters.FilterCriteria();
-
         criteria.setAttributeName(attrName);
-        criteria.setOperator(SearchParameters.Operator.NOT_EMPTY);
+        criteria.setOperator(SearchParameters.Operator.NOT_NULL);
 
         SearchParameters.FilterCriteria entityFilters = new 
SearchParameters.FilterCriteria();
-
         
entityFilters.setCondition(SearchParameters.FilterCriteria.Condition.OR);
         entityFilters.setCriterion(Collections.singletonList(criteria));
 
         SearchParameters searchParameters = new SearchParameters();
-
         
searchParameters.setTypeName(String.join(SearchContext.TYPENAME_DELIMITER, 
applicableTypes));
         searchParameters.setExcludeDeletedEntities(true);
         searchParameters.setIncludeSubClassifications(false);
         searchParameters.setEntityFilters(entityFilters);
-        searchParameters.setAttributes(Collections.singleton(attrName));
-        searchParameters.setOffset(0);
+        searchParameters.setAttributes(Collections.emptySet());
         searchParameters.setLimit(1);
 
         AtlasSearchResult atlasSearchResult = 
entityDiscoveryService.searchWithParameters(searchParameters);
+        return atlasSearchResult != null && 
CollectionUtils.isNotEmpty(atlasSearchResult.getEntities());
+    }
+
+    private boolean isBusinessAttributePresentInGraph(String 
vertexPropertyName, Set<String> applicableTypes) {
+        if (graph == null || CollectionUtils.isEmpty(applicableTypes)) {
+            return false;
+        }
+
+        try {
+            for (String typeName : applicableTypes) {
 
-        return CollectionUtils.isNotEmpty(atlasSearchResult.getEntities());
+                Iterable<AtlasVertex> vertices = graph.query()
+                        .has(Constants.ENTITY_TYPE_PROPERTY_KEY, typeName)
+                        .has(vertexPropertyName, 
AtlasGraphQuery.ComparisionOperator.NOT_EQUAL, (Object) null)
+                        .has(Constants.STATE_PROPERTY_KEY, 
AtlasEntity.Status.ACTIVE.name())

Review Comment:
   Is it necessary to check only for ACTIVE entities? The business metadata 
can't be deleted even when a deleted entity has reference to it, right?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to