UmeshPatil-1 commented on code in PR #490:
URL: https://github.com/apache/atlas/pull/490#discussion_r2939668565
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -363,44 +411,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 :
businessMetadataDef.getAttributeDefs()) {
+ validateAttributeReferences(businessMetadataDef, attributeDef);
+ }
+ }
- 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);
+ 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.isNotEmpty(applicableTypes) &&
isBusinessAttributePresent(vertexPropertyName, applicableTypes)) {
- throw new
AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName);
- }
- }
+ if (CollectionUtils.isEmpty(applicableTypes)) {
+ return;
+ }
+
+ Set<String> allApplicableTypes =
getApplicableTypesWithSubTypes(applicableTypes);
+ String qualifiedName =
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(bmDef,
attributeDef.getName());
+ String vertexPropertyName =
AtlasStructType.AtlasAttribute.generateVertexPropertyName(bmDef, attributeDef,
qualifiedName);
+
+ long startTime = System.currentTimeMillis();
+
+ boolean isPresent =
isBusinessAttributePresentInGraph(vertexPropertyName, allApplicableTypes);
+
+ 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);
+ private boolean isBusinessAttributePresentInGraph(String
vertexPropertyName, Set<String> allApplicableTypes) {
+ if (graph == null || CollectionUtils.isEmpty(allApplicableTypes)) {
+ return false;
+ }
- SearchParameters.FilterCriteria entityFilters = new
SearchParameters.FilterCriteria();
+ try {
+ List<String> typesList = new ArrayList<>(allApplicableTypes);
-
entityFilters.setCondition(SearchParameters.FilterCriteria.Condition.OR);
- entityFilters.setCriterion(Collections.singletonList(criteria));
+ // 1. To Check if the BM property exists on a vertex where the
direct type matches
+ Iterable<AtlasVertex> verticesDirect = graph.query()
+ .has(vertexPropertyName,
AtlasGraphQuery.ComparisionOperator.NOT_EQUAL, (Object) null)
+ .in(Constants.ENTITY_TYPE_PROPERTY_KEY, typesList)
+ .vertices();
- SearchParameters searchParameters = new SearchParameters();
+ if (verticesDirect != null && verticesDirect.iterator().hasNext())
{
+ return true;
+ }
-
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.setLimit(1);
+ // 2. To Check if the BM property exists on a vertex where it
inherits from one of parent Types
+ // This is crucial for Case 6 (Parent -> Child)
+ Iterable<AtlasVertex> verticesInherited = graph.query()
+ .has(vertexPropertyName,
AtlasGraphQuery.ComparisionOperator.NOT_EQUAL, (Object) null)
+ .in(Constants.SUPER_TYPES_PROPERTY_KEY, typesList)
+ .vertices();
- AtlasSearchResult atlasSearchResult =
entityDiscoveryService.searchWithParameters(searchParameters);
+ if (verticesInherited != null &&
verticesInherited.iterator().hasNext()) {
+ return true;
+ }
+ } catch (Exception e) {
+ LOG.error("Error occurred while querying graph for references of
property: {}", vertexPropertyName, e);
+ return true;
+ }
+ return false;
+ }
Review Comment:
Two targeted predicates are clearer and typically index-friendlier than a
broad OR query across different properties, It will hurts readability and
observability of failure points.
--
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]
