sheetalshah1007 commented on code in PR #490:
URL: https://github.com/apache/atlas/pull/490#discussion_r3007281194
##########
webapp/src/main/java/org/apache/atlas/web/rest/TypesREST.java:
##########
Review Comment:
**Bulk REST delete has no force**
`DELETE` `/api/atlas/v2/types/typedefs` still calls
`typeDefStore.deleteTypesDef(typesDef)` (always `forceDelete = false`
internally). **Single-name** delete supports `?force=true`, but **bulk** does
not.
Suggestion: add an optional force query param to `deleteAtlasTypeDefs` and
align bulk delete with `force`
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -360,47 +405,104 @@ private AtlasBusinessMetadataDef
toBusinessMetadataDef(AtlasVertex vertex) throw
return ret;
}
- private void checkBusinessMetadataRef(String typeName) throws
AtlasBaseException {
- AtlasBusinessMetadataDef businessMetadataDef =
typeRegistry.getBusinessMetadataDefByName(typeName);
+ private void checkBusinessMetadataRef(AtlasBusinessMetadataDef
businessMetadataDef, String identifier) throws AtlasBaseException {
+ if (businessMetadataDef == null ||
CollectionUtils.isEmpty(businessMetadataDef.getAttributeDefs())) {
+ return;
+ }
- if (businessMetadataDef != null) {
- List<AtlasStructDef.AtlasAttributeDef> attributeDefs =
businessMetadataDef.getAttributeDefs();
+ AtlasBusinessMetadataType bmType =
typeRegistry.getBusinessMetadataTypeByName(businessMetadataDef.getName());
- 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()) {
+ Set<String> allApplicableTypes;
- if (CollectionUtils.isNotEmpty(applicableTypes) &&
isBusinessAttributePresent(vertexPropertyName, applicableTypes)) {
- throw new
AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName);
- }
+ if (bmType != null) {
+ AtlasBusinessMetadataType.AtlasBusinessAttribute bmAttr =
+ (AtlasBusinessMetadataType.AtlasBusinessAttribute)
bmType.getAttribute(attributeDef.getName());
+ Set<String> cachedTypes = bmAttr != null ?
bmAttr.getApplicableEntityTypeNamesWithSubTypes() : Collections.emptySet();
+
+ allApplicableTypes = CollectionUtils.isNotEmpty(cachedTypes) ?
cachedTypes : getApplicableTypesWithSubTypes(attributeDef);
+ } else {
+ allApplicableTypes =
getApplicableTypesWithSubTypes(attributeDef);
}
+
+ LOG.info("REF-CHECK [ApplicableTypes] BM='{}' attr='{}' types={}",
+ businessMetadataDef.getName(), attributeDef.getName(),
allApplicableTypes);
+
+ if (CollectionUtils.isEmpty(allApplicableTypes)) {
+ continue;
+ }
+
+ validateAttributeReferences(businessMetadataDef, attributeDef,
allApplicableTypes, identifier);
}
}
- private boolean isBusinessAttributePresent(String attrName, Set<String>
applicableTypes) throws AtlasBaseException {
- SearchParameters.FilterCriteria criteria = new
SearchParameters.FilterCriteria();
+ private void validateAttributeReferences(AtlasBusinessMetadataDef bmDef,
AtlasStructDef.AtlasAttributeDef attributeDef,
+ Set<String> allApplicableTypes,
String identifier) throws AtlasBaseException {
+ String qualifiedName =
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(bmDef,
attributeDef.getName());
+ String vertexPropertyName =
AtlasStructType.AtlasAttribute.generateVertexPropertyName(bmDef, attributeDef,
qualifiedName);
+ boolean isPresent =
isBusinessAttributePresentInGraph(vertexPropertyName, allApplicableTypes);
- criteria.setAttributeName(attrName);
- criteria.setOperator(SearchParameters.Operator.NOT_EMPTY);
+ if (isPresent) {
+ LOG.error("Cannot delete BusinessMetadata '{}' (request='{}') -
attribute '{}' (vertex property: '{}') has references in entity types: {}",
+ bmDef.getName(), identifier, attributeDef.getName(),
vertexPropertyName, allApplicableTypes);
+ throw new AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES,
bmDef.getName());
+ }
+ }
- SearchParameters.FilterCriteria entityFilters = new
SearchParameters.FilterCriteria();
+ private boolean isBusinessAttributePresentInGraph(String
vertexPropertyName, Set<String> allApplicableTypes) throws AtlasBaseException {
+ if (CollectionUtils.isEmpty(allApplicableTypes)) {
+ return false;
+ }
-
entityFilters.setCondition(SearchParameters.FilterCriteria.Condition.OR);
- entityFilters.setCriterion(Collections.singletonList(criteria));
+ try {
+ List<String> typesList = new ArrayList<>(allApplicableTypes);
- SearchParameters searchParameters = new SearchParameters();
+ // Single combined query
+ AtlasGraphQuery query = graph.query()
+ .has(vertexPropertyName,
AtlasGraphQuery.ComparisionOperator.NOT_EQUAL, (Object) null);
-
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);
+ List<AtlasGraphQuery> orConditions = new ArrayList<>();
Review Comment:
Remove the inline comments here—the code is clear enough without them
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasTypeDefGraphStoreV2.java:
##########
@@ -73,14 +73,11 @@ public class AtlasTypeDefGraphStoreV2 extends
AtlasTypeDefGraphStore {
protected final AtlasGraph atlasGraph;
- private final EntityDiscoveryService entityDiscoveryService;
-
@Inject
public AtlasTypeDefGraphStoreV2(AtlasTypeRegistry typeRegistry,
List<TypeDefChangeListener> typeDefChangeListeners, AtlasGraph atlasGraph,
EntityDiscoveryService entityDiscoveryService) {
super(typeRegistry, typeDefChangeListeners);
- this.atlasGraph = atlasGraph;
- this.entityDiscoveryService = entityDiscoveryService;
+ this.atlasGraph = atlasGraph;
LOG.debug("<== AtlasTypeDefGraphStoreV1()");
Review Comment:
AtlasTypeDefGraphStoreV2 still logs "AtlasTypeDefGraphStoreV1()"
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -360,47 +405,104 @@ private AtlasBusinessMetadataDef
toBusinessMetadataDef(AtlasVertex vertex) throw
return ret;
}
- private void checkBusinessMetadataRef(String typeName) throws
AtlasBaseException {
- AtlasBusinessMetadataDef businessMetadataDef =
typeRegistry.getBusinessMetadataDefByName(typeName);
+ private void checkBusinessMetadataRef(AtlasBusinessMetadataDef
businessMetadataDef, String identifier) throws AtlasBaseException {
+ if (businessMetadataDef == null ||
CollectionUtils.isEmpty(businessMetadataDef.getAttributeDefs())) {
+ return;
+ }
- if (businessMetadataDef != null) {
- List<AtlasStructDef.AtlasAttributeDef> attributeDefs =
businessMetadataDef.getAttributeDefs();
+ AtlasBusinessMetadataType bmType =
typeRegistry.getBusinessMetadataTypeByName(businessMetadataDef.getName());
- 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()) {
+ Set<String> allApplicableTypes;
- if (CollectionUtils.isNotEmpty(applicableTypes) &&
isBusinessAttributePresent(vertexPropertyName, applicableTypes)) {
- throw new
AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName);
- }
+ if (bmType != null) {
+ AtlasBusinessMetadataType.AtlasBusinessAttribute bmAttr =
+ (AtlasBusinessMetadataType.AtlasBusinessAttribute)
bmType.getAttribute(attributeDef.getName());
+ Set<String> cachedTypes = bmAttr != null ?
bmAttr.getApplicableEntityTypeNamesWithSubTypes() : Collections.emptySet();
+
+ allApplicableTypes = CollectionUtils.isNotEmpty(cachedTypes) ?
cachedTypes : getApplicableTypesWithSubTypes(attributeDef);
+ } else {
+ allApplicableTypes =
getApplicableTypesWithSubTypes(attributeDef);
}
+
+ LOG.info("REF-CHECK [ApplicableTypes] BM='{}' attr='{}' types={}",
+ businessMetadataDef.getName(), attributeDef.getName(),
allApplicableTypes);
+
+ if (CollectionUtils.isEmpty(allApplicableTypes)) {
+ continue;
+ }
+
+ validateAttributeReferences(businessMetadataDef, attributeDef,
allApplicableTypes, identifier);
}
}
- private boolean isBusinessAttributePresent(String attrName, Set<String>
applicableTypes) throws AtlasBaseException {
- SearchParameters.FilterCriteria criteria = new
SearchParameters.FilterCriteria();
+ private void validateAttributeReferences(AtlasBusinessMetadataDef bmDef,
AtlasStructDef.AtlasAttributeDef attributeDef,
+ Set<String> allApplicableTypes,
String identifier) throws AtlasBaseException {
+ String qualifiedName =
AtlasStructType.AtlasAttribute.getQualifiedAttributeName(bmDef,
attributeDef.getName());
+ String vertexPropertyName =
AtlasStructType.AtlasAttribute.generateVertexPropertyName(bmDef, attributeDef,
qualifiedName);
+ boolean isPresent =
isBusinessAttributePresentInGraph(vertexPropertyName, allApplicableTypes);
- criteria.setAttributeName(attrName);
- criteria.setOperator(SearchParameters.Operator.NOT_EMPTY);
+ if (isPresent) {
+ LOG.error("Cannot delete BusinessMetadata '{}' (request='{}') -
attribute '{}' (vertex property: '{}') has references in entity types: {}",
Review Comment:
Logging ERROR for existing references is misleading. Use **WARN** (or INFO)
and reserve ERROR for real failures.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -360,47 +405,104 @@ private AtlasBusinessMetadataDef
toBusinessMetadataDef(AtlasVertex vertex) throw
return ret;
}
- private void checkBusinessMetadataRef(String typeName) throws
AtlasBaseException {
- AtlasBusinessMetadataDef businessMetadataDef =
typeRegistry.getBusinessMetadataDefByName(typeName);
+ private void checkBusinessMetadataRef(AtlasBusinessMetadataDef
businessMetadataDef, String identifier) throws AtlasBaseException {
+ if (businessMetadataDef == null ||
CollectionUtils.isEmpty(businessMetadataDef.getAttributeDefs())) {
+ return;
+ }
- if (businessMetadataDef != null) {
- List<AtlasStructDef.AtlasAttributeDef> attributeDefs =
businessMetadataDef.getAttributeDefs();
+ AtlasBusinessMetadataType bmType =
typeRegistry.getBusinessMetadataTypeByName(businessMetadataDef.getName());
- 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()) {
+ Set<String> allApplicableTypes;
- if (CollectionUtils.isNotEmpty(applicableTypes) &&
isBusinessAttributePresent(vertexPropertyName, applicableTypes)) {
- throw new
AtlasBaseException(AtlasErrorCode.TYPE_HAS_REFERENCES, typeName);
- }
+ if (bmType != null) {
+ AtlasBusinessMetadataType.AtlasBusinessAttribute bmAttr =
+ (AtlasBusinessMetadataType.AtlasBusinessAttribute)
bmType.getAttribute(attributeDef.getName());
+ Set<String> cachedTypes = bmAttr != null ?
bmAttr.getApplicableEntityTypeNamesWithSubTypes() : Collections.emptySet();
+
+ allApplicableTypes = CollectionUtils.isNotEmpty(cachedTypes) ?
cachedTypes : getApplicableTypesWithSubTypes(attributeDef);
+ } else {
+ allApplicableTypes =
getApplicableTypesWithSubTypes(attributeDef);
}
+
+ LOG.info("REF-CHECK [ApplicableTypes] BM='{}' attr='{}' types={}",
Review Comment:
logs **INFO** for every attribute's applicable types
Suggestion: prefer DEBUG logging for this
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasTypeDefGraphStoreV2.java:
##########
@@ -73,14 +73,11 @@ public class AtlasTypeDefGraphStoreV2 extends
AtlasTypeDefGraphStore {
protected final AtlasGraph atlasGraph;
- private final EntityDiscoveryService entityDiscoveryService;
-
@Inject
public AtlasTypeDefGraphStoreV2(AtlasTypeRegistry typeRegistry,
List<TypeDefChangeListener> typeDefChangeListeners, AtlasGraph atlasGraph,
EntityDiscoveryService entityDiscoveryService) {
Review Comment:
The constructor still takes `EntityDiscoveryService entityDiscoveryService
`but no longer stores or uses it:
Remove the parameter to fix the unwanted wiring
--
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]
