sheetalshah1007 commented on code in PR #490:
URL: https://github.com/apache/atlas/pull/490#discussion_r2650544323
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -363,44 +368,71 @@ 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()) {
Review Comment:
Extract the attribute iteration logic in `checkBusinessMetadataRef` to a
private method (e.g., `validateAttributeReferences`) for better readability.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -363,44 +368,71 @@ 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()) {
+ 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)) {
+ continue;
+ }
+
+ String qualifiedName =
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(businessMetadataDef,
attributeDef.getName());
+ String vertexPropertyName =
AtlasStructType.AtlasAttribute.generateVertexPropertyName(businessMetadataDef,
attributeDef, qualifiedName);
+
+ boolean isPresent;
+
+ if (Boolean.TRUE.equals(attributeDef.getIsIndexable())) {
+ isPresent = isBusinessAttributePresent(vertexPropertyName,
applicableTypes);
+ } else {
+ isPresent =
isBusinessAttributePresentInGraph(vertexPropertyName, applicableTypes);
+ }
+
+ if (isPresent) {
+ throw new
AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName);
}
}
}
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) {
Review Comment:
**1. Replace manual iteration with Graph Query for better performance:**
The method isBusinessAttributePresentInGraph manually iterates over all
vertices of each applicable type using
`graph.getVertices(Constants.ENTITY_TYPE_PROPERTY_KEY, typeName), `then checks
each vertex for the property and state. This can be slow for large graphs, as
it loads and inspects every vertex in memory. Consider replacing manual
iteration with Graph Query for better performance.
**2. Add error handling and logging:**
No exception handling in the graph operations.
If the graph is unavailable or queries fail, it could lead to unhandled
exceptions. Wrap the query in a try-catch and log warnings/errors.
Also, add optional performance logging to monitor query times in production.
**3. Add JUnit tests for the new method covering:**
-Non-indexable attributes with/without assignments.
-Inactive entities (should not count).
--
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]
