sheetalshah1007 commented on code in PR #490:
URL: https://github.com/apache/atlas/pull/490#discussion_r2832518690
##########
client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java:
##########
@@ -353,7 +353,19 @@ public void deleteAtlasTypeDefs(AtlasTypesDef typesDef)
throws AtlasServiceExcep
}
public void deleteTypeByName(String typeName) throws AtlasServiceException
{
- callAPI(API_V2.DELETE_TYPE_DEF_BY_NAME, (Class) null, null, typeName);
+ callAPI(API_V2.DELETE_TYPE_DEF_BY_NAME, (Class<?>) null, null,
typeName);
Review Comment:
The if-else with duplicate API calls can be avoided; consider combining the
if-else logic since both overloaded methods call the same API - just with
different query params.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -271,15 +285,29 @@ public AtlasVertex preDeleteByName(String name) throws
AtlasBaseException {
throw new AtlasBaseException(AtlasErrorCode.TYPE_NAME_NOT_FOUND,
name);
}
- checkBusinessMetadataRef(existingDef.getName());
+ if (!forceDelete) {
+ boolean hasIndexableAttribute = hasIndexableAttribute(existingDef);
- LOG.debug("<== AtlasBusinessMetadataDefStoreV2.preDeleteByName({}):
{}", name, ret);
+ if (!hasIndexableAttribute) {
Review Comment:
The hasIndexableAttribute check could be expensive if there are many
attributes.
Consider caching this in the BusinessMetadataDef or TypeRegistry.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -271,15 +285,29 @@ public AtlasVertex preDeleteByName(String name) throws
AtlasBaseException {
throw new AtlasBaseException(AtlasErrorCode.TYPE_NAME_NOT_FOUND,
name);
}
- checkBusinessMetadataRef(existingDef.getName());
+ if (!forceDelete) {
Review Comment:
The logic at lines 288-296 and 322-330 (in preDeleteByGuid) is duplicated.
Extract to a helper method: validateDeletion(AtlasBusinessMetadataDef
def, boolean forceDelete)
##########
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();
Review Comment:
Consider adding a limit (e.g., .limit(1)) since we only need to know if ANY
exist
##########
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) {
Review Comment:
**CRITICAL:** Catching all exceptions and returning true treats temporary
system errors (network issues, DB timeouts) the same as actual reference
blockers. Any temporary failure makes deletion fail with
"`TYPE_HAS_REFERENCES`" error - even when no references exist. Users can't
tell if they need to clean up references or just retry. Throw an exception with
a clear "system error" message instead of silently returning true.
##########
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)) {
Review Comment:
**CRITICAL**: Remove the graph null check at line 451. Returning false when
graph is null means "safe to delete" without actually verifying references -
this is dangerous. AtlasGraph is dependency-injected and cannot be null in
production.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -258,8 +266,14 @@ public AtlasBusinessMetadataDef updateByGuid(String guid,
AtlasBusinessMetadataD
return ret;
}
+ @Override
public AtlasVertex preDeleteByName(String name) throws AtlasBaseException {
- LOG.debug("==> AtlasBusinessMetadataDefStoreV2.preDeleteByName({})",
name);
+ return preDeleteByName(name, false);
+ }
+
+ @Override
+ public AtlasVertex preDeleteByName(String name, boolean forceDelete)
throws AtlasBaseException {
+ LOG.debug("==> AtlasBusinessMetadataDefStoreV2.preDeleteByName({},
{})", name, forceDelete);
AtlasBusinessMetadataDef existingDef =
typeRegistry.getBusinessMetadataDefByName(name);
Review Comment:
Add a warning log when force-delete is used; something like "Force-deleting
BusinessMetadata '{}'. Skipping validation - orphaned references may remain.",
name"
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -291,15 +319,35 @@ public AtlasVertex preDeleteByGuid(String guid) throws
AtlasBaseException {
throw new AtlasBaseException(AtlasErrorCode.TYPE_GUID_NOT_FOUND,
guid);
}
- if (existingDef != null) {
+ if (existingDef != null && !forceDelete) {
+ boolean hasIndexableAttribute = hasIndexableAttribute(existingDef);
+
+ if (!hasIndexableAttribute) {
Review Comment:
The hasIndexableAttribute check could be expensive if there are many
attributes.
Consider caching this in the BusinessMetadataDef or TypeRegistry.
##########
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java:
##########
@@ -257,7 +257,9 @@ public enum AtlasErrorCode {
IMPORT_REGISTRATION_FAILED(500, "ATLAS-500-00-020", "Failed to register
import request"),
IMPORT_FAILED(500, "ATLAS-500-00-021", "Import with id {0} failed"),
ABORT_IMPORT_FAILED(500, "ATLAS-500-00-022", "Failed to abort import with
id {0}"),
- IMPORT_QUEUEING_FAILED(500, "ATLAS-500-00-023", "Failed to add import with
id {0} to request queue, please try again later");
+ IMPORT_QUEUEING_FAILED(500, "ATLAS-500-00-023", "Failed to add import with
id {0} to request queue, please try again later"),
+
+ NON_INDEXABLE_BM_DELETE_NOT_ALLOWED(400, "ATLAS-400-00-106", "Deletion not
allowed for non-indexable Business Metadata ''{0}'' without force-delete.
Please use the force-delete parameter to remove.");
Review Comment:
the message could be more user-friendly by explaining WHAT force-delete does.
Suggested message:
"Deletion not allowed for non-indexable Business Metadata ''{0}'' without
force parameter.
Non-indexable attributes cannot be efficiently validated for references. Use
force=true to
skip validation and delete (Warning: may leave orphaned references)."
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -271,15 +285,29 @@ public AtlasVertex preDeleteByName(String name) throws
AtlasBaseException {
throw new AtlasBaseException(AtlasErrorCode.TYPE_NAME_NOT_FOUND,
name);
}
- checkBusinessMetadataRef(existingDef.getName());
+ if (!forceDelete) {
+ boolean hasIndexableAttribute = hasIndexableAttribute(existingDef);
- LOG.debug("<== AtlasBusinessMetadataDefStoreV2.preDeleteByName({}):
{}", name, ret);
+ if (!hasIndexableAttribute) {
+ LOG.warn("Deletion blocked for non-indexable Business Metadata
'{}' without force-delete flag", name);
+ throw new
AtlasBaseException(AtlasErrorCode.NON_INDEXABLE_BM_DELETE_NOT_ALLOWED, name);
+ }
+ checkBusinessMetadataRef(existingDef.getName());
+ }
+
+ LOG.debug("<== AtlasBusinessMetadataDefStoreV2.preDeleteByName({},
{}): {}", name, forceDelete, ret);
return ret;
}
+ @Override
public AtlasVertex preDeleteByGuid(String guid) throws AtlasBaseException {
- LOG.debug("==> AtlasBusinessMetadataDefStoreV2.preDeleteByGuid({})",
guid);
+ return preDeleteByGuid(guid, false);
+ }
+
+ @Override
+ public AtlasVertex preDeleteByGuid(String guid, boolean forceDelete)
throws AtlasBaseException {
+ LOG.debug("==> AtlasBusinessMetadataDefStoreV2.preDeleteByGuid({},
{})", guid, forceDelete);
AtlasBusinessMetadataDef existingDef =
typeRegistry.getBusinessMetadataDefByGuid(guid);
Review Comment:
Add a warning log when force-delete is used; something like "Force-deleting
BusinessMetadata '{}'. Skipping validation - orphaned references may remain.",
name"
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasDefStore.java:
##########
@@ -45,9 +45,25 @@ public interface AtlasDefStore<T extends AtlasBaseTypeDef> {
AtlasVertex preDeleteByName(String name) throws AtlasBaseException;
- void deleteByName(String name, AtlasVertex preDeleteResult) throws
AtlasBaseException;
-
AtlasVertex preDeleteByGuid(String guid) throws AtlasBaseException;
+ void deleteByName(String name, AtlasVertex preDeleteResult) throws
AtlasBaseException;
+
void deleteByGuid(String guid, AtlasVertex preDeleteResult) throws
AtlasBaseException;
+
+ default AtlasVertex preDeleteByName(String name, boolean forceDelete)
throws AtlasBaseException {
+ return preDeleteByName(name);
+ }
+
+ default void deleteByName(String name, AtlasVertex preDeleteResult,
boolean forceDelete) throws AtlasBaseException {
+ deleteByName(name, preDeleteResult);
+ }
+
+ default AtlasVertex preDeleteByGuid(String guid, boolean forceDelete)
throws AtlasBaseException {
+ return preDeleteByGuid(guid);
+ }
+
+ default void deleteByGuid(String guid, AtlasVertex preDeleteResult,
boolean forceDelete) throws AtlasBaseException {
+ deleteByGuid(guid, preDeleteResult);
+ }
}
Review Comment:
Good use of default methods for backward compatibility.
SUGGESTION: Add debug logging when force-delete flag is ignored.
Force-delete only applies to BusinessMetadata (fixes slow array deletion); the
other 5 type stores (Enum, Struct, Classification, Entity, Relationship) don't
need it, so these default methods just ignore the flag - which is fine.
Adding a log like "Force-delete flag ignored for {typeName} -
BusinessMetadata-only feature" clarifies the design and helps debug deletion
issues without any breaking changes.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -291,15 +319,35 @@ public AtlasVertex preDeleteByGuid(String guid) throws
AtlasBaseException {
throw new AtlasBaseException(AtlasErrorCode.TYPE_GUID_NOT_FOUND,
guid);
}
- if (existingDef != null) {
+ if (existingDef != null && !forceDelete) {
+ boolean hasIndexableAttribute = hasIndexableAttribute(existingDef);
+
+ if (!hasIndexableAttribute) {
+ LOG.warn("Deletion blocked for non-indexable Business Metadata
'{}' without force-delete flag", existingDef.getName());
+ throw new
AtlasBaseException(AtlasErrorCode.NON_INDEXABLE_BM_DELETE_NOT_ALLOWED,
existingDef.getName());
+ }
checkBusinessMetadataRef(existingDef.getName());
}
- LOG.debug("<== AtlasBusinessMetadataDefStoreV2.preDeleteByGuid({}):
ret={}", guid, ret);
+ LOG.debug("<== AtlasBusinessMetadataDefStoreV2.preDeleteByGuid({},
{}): ret={}", guid, forceDelete, ret);
return ret;
}
+ private boolean hasIndexableAttribute(AtlasBusinessMetadataDef bmDef) {
+ if (bmDef == null ||
CollectionUtils.isEmpty(bmDef.getAttributeDefs())) {
+ return false;
+ }
+
+ for (AtlasStructDef.AtlasAttributeDef attributeDef :
bmDef.getAttributeDefs()) {
+ if (attributeDef.getIsIndexable()) {
Review Comment:
****CRITICAL**: Logic is inverted - checks for good attributes instead of
problematic ones.**
Example: BM with attr1 (String, indexable) + attr2 (Array, non-indexable)
- Current: hasIndexableAttribute() = true → normal delete allowed → SLOW
(still checks array)
- Should be: **hasNonIndexableAttribute() = true → force-delete required**
The check should block normal deletion if ANY attribute is non-indexable,
not only if ALL attributes are non-indexable.
##########
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();
Review Comment:
Consider adding a limit (e.g., .limit(1)) since we only need to know if ANY
exist
##########
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());
Review Comment:
Current problem: Error "TYPE_HAS_REFERENCES" doesn't say which attribute is
blocking deletion or where to look.
Suggestion: Add detailed logging before throwing the error for easier
debugging.
```
LOG.error("Cannot delete BusinessMetadata '{}' - attribute '{}' (vertex
property: '{}')
has references in entity types: {}", bmDef.getName(),
attributeDef.getName(),
vertexPropertyName, allApplicableTypes);
```
This way logs would show exactly which attribute, property name, and entity
types have references.
Using this information, user can run a targeted query and find the exact
references
##########
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;
+ }
+
+ private Set<String> getApplicableTypesWithSubTypes(Set<String>
applicableTypes) throws AtlasBaseException {
Review Comment:
Suggestions:
1. Add JavaDoc explaining WHY sub-types are needed as this is an important
business logic - without JavaDoc, developers won't understand the inheritance
scenario.
2. Consider caching the result in TypeRegistry to avoid repeated calculations
##########
webapp/src/test/java/org/apache/atlas/web/rest/TypesRESTTest.java:
##########
Review Comment:
Missing test coverage: Test the actual use case - non-indexable BM deletion
##########
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);
Review Comment:
`getApplicableTypesWithSubTypes() `gets called multiple times for the same
BM definition.
Optimization:
Calculate `allApplicableTypes` once at the BM level and pass it down:
- Move `getApplicableTypesWithSubTypes` call to `checkBusinessMetadataRef`
- Pass the expanded types to `validateAttributeReferences`
##########
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:
The two queries (for direct and inherited type match) could be combined
into one to make it more efficient
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java:
##########
@@ -539,14 +539,20 @@ public AtlasTypesDef createUpdateTypesDef(AtlasTypesDef
typesToCreate, AtlasType
@Override
@GraphTransaction
public void deleteTypesDef(AtlasTypesDef typesDef) throws
AtlasBaseException {
+ deleteTypesDef(typesDef, false);
+ }
+
+ @GraphTransaction
+ public void deleteTypesDef(AtlasTypesDef typesDef, boolean forceDelete)
throws AtlasBaseException {
if (LOG.isDebugEnabled()) {
Review Comment:
LOG.isDebugEnabled() check can be removed
##########
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);
Review Comment:
Code/logging related to time capture can be removed
--
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]
