pinal-shah commented on code in PR #490:
URL: https://github.com/apache/atlas/pull/490#discussion_r3185890054
##########
webapp/src/main/java/org/apache/atlas/web/rest/TypesREST.java:
##########
@@ -399,25 +401,34 @@ public AtlasTypesDef updateAtlasTypeDefs(final
AtlasTypesDef typesDef) throws At
}
/**
- * Bulk delete API for all types
- * @param typesDef A composite object that captures all types to be deleted
+ * Bulk delete API for all types.
+ *
+ * @param typesDef A composite object that captures all types to be
deleted.
+ * @param forceDelete If true, bypasses pre-delete validation checks and
forcefully removes type definitions.
Review Comment:
Please mention, only for businessMetadata typdef
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasAbstractDefStoreV2.java:
##########
@@ -148,24 +148,46 @@ public boolean isValidName(String typeName) {
@Override
public void deleteByName(String name, AtlasVertex preDeleteResult) throws
AtlasBaseException {
- LOG.debug("==> AtlasAbstractDefStoreV1.deleteByName({}, {})", name,
preDeleteResult);
+ LOG.debug("==> AtlasAbstractDefStoreV2.deleteByName({}, {})", name,
preDeleteResult);
AtlasVertex vertex = (preDeleteResult == null) ? preDeleteByName(name)
: preDeleteResult;
typeDefStore.deleteTypeVertex(vertex);
- LOG.debug("<== AtlasAbstractDefStoreV1.deleteByName({}, {})", name,
preDeleteResult);
+ LOG.debug("<== AtlasAbstractDefStoreV2.deleteByName({}, {})", name,
preDeleteResult);
}
@Override
public void deleteByGuid(String guid, AtlasVertex preDeleteResult) throws
AtlasBaseException {
Review Comment:
@Override
public void deleteByGuid(String guid, AtlasVertex preDeleteResult)
throws AtlasBaseException {
deleteByGuid(guid, preDeleteResult, false);
}
Please review above
##########
intg/src/main/java/org/apache/atlas/type/AtlasBusinessMetadataType.java:
##########
@@ -137,6 +151,29 @@ void resolveReferencesPhase2(AtlasTypeRegistry
typeRegistry) throws AtlasBaseExc
}
}
+ @Override
+ void resolveReferencesPhase3(AtlasTypeRegistry typeRegistry) throws
AtlasBaseException {
Review Comment:
please review if this can be achieved in resolveReferencesPhase2
##########
webapp/src/main/java/org/apache/atlas/web/rest/TypesREST.java:
##########
@@ -426,22 +437,30 @@ public void deleteAtlasTypeDefs(final AtlasTypesDef
typesDef) throws AtlasBaseEx
/**
* Delete API for type identified by its name.
* @param typeName Name of the type to be deleted.
+ * @param forceDelete If true, bypasses pre-delete validation checks and
forcefully removes the type definition.
Review Comment:
Please add, only supported for businessMetadata
##########
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java:
##########
@@ -183,6 +183,7 @@ public enum AtlasErrorCode {
BLANK_NAME_ATTRIBUTE(400, "ATLAS-400-00-104", "Name Attribute can't be
empty!"),
BLANK_VALUE_ATTRIBUTE(400, "ATLAS-400-00-105", "Value Attribute can't be
empty!"),
INVALID_RELATIONSHIP_LABEL(400, "ATLAS-400-00-106", "Invalid relationship
label {0}. The referenced entity type {1} could not be resolved from the type
registry."),
+ NON_INDEXABLE_BM_DELETE_NOT_ALLOWED(400, "ATLAS-400-00-107", "Deletion not
allowed for non-indexable Business Metadata ''{0}'' without force=true.
Non-indexable attributes cannot be validated efficiently for references; use
force=true to skip validation and delete (warning: orphaned references may
remain)."),
Review Comment:
What do you mean by orphaned references may remain?
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasDefStore.java:
##########
@@ -45,9 +49,33 @@ 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 {
+ if (forceDelete) {
+ LOG.debug("Force-delete flag ignored in {}.preDeleteByName() for
type '{}'; feature is implemented only for BusinessMetadata.",
getClass().getSimpleName(), name);
Review Comment:
no need to log here
##########
intg/src/main/java/org/apache/atlas/type/AtlasBusinessMetadataType.java:
##########
@@ -115,10 +123,16 @@ void resolveReferences(AtlasTypeRegistry typeRegistry)
throws AtlasBaseException
bmAttribute = new AtlasBusinessAttribute(attribute,
entityTypes);
}
+ if (!hasNonIndexableAttributes &&
!Boolean.TRUE.equals(attributeDef.getIsIndexable())) {
Review Comment:
Simplify !Boolean.TRUE.equals(attributeDef.getIsIndexable() to ->
!attributeDef.getIsIndexable()
Please incoporate other places as well
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasBusinessMetadataDefStoreV2.java:
##########
@@ -291,15 +302,49 @@ public AtlasVertex preDeleteByGuid(String guid) throws
AtlasBaseException {
throw new AtlasBaseException(AtlasErrorCode.TYPE_GUID_NOT_FOUND,
guid);
}
- if (existingDef != null) {
- checkBusinessMetadataRef(existingDef.getName());
- }
+ validateDeletion(existingDef, forceDelete, guid);
- LOG.debug("<== AtlasBusinessMetadataDefStoreV2.preDeleteByGuid({}):
ret={}", guid, ret);
+ LOG.debug("<== AtlasBusinessMetadataDefStoreV2.preDeleteByGuid({},
{}): ret={}", guid, forceDelete, ret);
return ret;
}
+ private void validateDeletion(AtlasBusinessMetadataDef
businessMetadataDef, boolean forceDelete, String identifier) throws
AtlasBaseException {
+ if (businessMetadataDef == null) {
+ return;
+ }
+
+ if (forceDelete) {
+ LOG.warn("Force-deleting BusinessMetadata '{}'. Skipping
validation - orphaned references may remain.", businessMetadataDef.getName());
+ return;
+ }
+
+ if (hasNonIndexableAttribute(businessMetadataDef)) {
+ LOG.warn("Deletion blocked for non-indexable Business Metadata
'{}' without force-delete flag", businessMetadataDef.getName());
+ throw new
AtlasBaseException(AtlasErrorCode.NON_INDEXABLE_BM_DELETE_NOT_ALLOWED,
businessMetadataDef.getName());
+ }
+
+ checkBusinessMetadataRef(businessMetadataDef, identifier);
+ }
+
+ private boolean hasNonIndexableAttribute(AtlasBusinessMetadataDef
businessMetadataDef) {
+ AtlasBusinessMetadataType bmType =
typeRegistry.getBusinessMetadataTypeByName(businessMetadataDef.getName());
+
+ if (bmType != null && bmType.hasNonIndexableAttributes()) {
+ return true;
+ }
+
+ if
(CollectionUtils.isNotEmpty(businessMetadataDef.getAttributeDefs())) {
Review Comment:
please review if this is required #337 to #343
--
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]
