-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(forms) Handle deleting forms references when hard deleting forms #10820
feat(forms) Handle deleting forms references when hard deleting forms #10820
Conversation
WalkthroughRecent updates to Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant DeleteEntityService as DeleteEntityService
participant EntitySearchService as EntitySearchService
participant EntityService as EntityService
participant GraphService as GraphService
Client->>DeleteEntityService: deleteReferencesTo(urn, dryRun)
alt dryRun is true
DeleteEntityService->>EntitySearchService: search assets referencing urn
DeleteEntityService->>Client: Return simulated deletion result
else dryRun is false
DeleteEntityService->>EntitySearchService: search assets referencing urn
loop Over found assets
DeleteEntityService->>EntityService: propose deletion of search references
EntityService->>DeleteEntityService: Return deletion result
end
DeleteEntityService->>GraphService: update graph relationships
DeleteEntityService->>Client: Return actual deletion result
end
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- metadata-io/src/test/java/com/linkedin/metadata/entity/DeleteEntityServiceTest.java (4 hunks)
- metadata-service/factories/src/main/java/com/linkedin/gms/factory/entity/DeleteEntityServiceFactory.java (2 hunks)
- metadata-service/services/src/main/java/com/linkedin/metadata/entity/DeleteEntityService.java (7 hunks)
Additional comments not posted (31)
metadata-service/factories/src/main/java/com/linkedin/gms/factory/entity/DeleteEntityServiceFactory.java (3)
7-7
: Import Approved.The import for
EntitySearchService
is necessary for the new functionality.
26-28
: Field Injection Approved.The field
_entitySearchService
is correctly annotated with@Autowired
and@Qualifier("entitySearchService")
.
33-33
: Method Update Approved.The method
createDeleteEntityService
correctly includes_entitySearchService
as a parameter.metadata-io/src/test/java/com/linkedin/metadata/entity/DeleteEntityServiceTest.java (10)
10-13
: Imports Approved.The imports for
FormAssociation
,FormAssociationArray
,FormVerificationAssociationArray
, andForms
are necessary for the new tests.
26-26
: Import Approved.The import for
Filter
is necessary for the new tests.
29-32
: Imports Approved.The imports for
EntitySearchService
,ScrollResult
,SearchEntity
, andSearchEntityArray
are necessary for the new tests.
36-36
: Import Approved.The import for
MetadataChangeProposal
is necessary for the new tests.
52-52
: Field Approved.The field
_mockSearchService
is necessary for the new tests.
58-58
: Mock Approved.The mock for
EntitySearchService
is necessary for the new tests.
65-65
: Constructor Update Approved.The
DeleteEntityService
constructor update to include_mockSearchService
is necessary for the new tests.
136-220
: New Test Method Approved.The new test method
testDeleteSearchReferences
is comprehensive and covers the functionality of deleting search references.
222-274
: New Test Method Approved.The new test method
testDeleteNoSearchReferences
is comprehensive and covers the functionality of not deleting search references when there are none.
276-333
: New Test Method Approved.The new test method
testDeleteSearchReferencesDryRun
is comprehensive and covers the functionality of dry-run deletion of search references.metadata-service/services/src/main/java/com/linkedin/metadata/entity/DeleteEntityService.java (18)
9-13
: Imports Approved.The imports for
FormAssociation
,FormAssociationArray
,FormVerificationAssociation
,FormVerificationAssociationArray
, andForms
are necessary for the new functionality.
30-35
: Imports Approved.The imports for
Condition
,ConjunctiveCriterion
,ConjunctiveCriterionArray
,Criterion
, andCriterionArray
are necessary for the new functionality.
40-42
: Imports Approved.The imports for
EntitySearchService
,ScrollResult
, andSearchEntity
are necessary for the new functionality.
47-47
: Import Approved.The import for
ArrayList
is necessary for the new functionality.
57-57
: Import Approved.The import for
Nullable
is necessary for the new functionality.
69-69
: Field Approved.The field
_searchService
is necessary for the new functionality.
72-72
: Constant Approved.The static constant
BATCH_SIZE
is necessary for the new functionality.
86-87
: Reminder: UpdateDeleteReferencesResponse
.The TODO comment indicates that
DeleteReferencesResponse
needs to be updated in the future.
90-93
: Method Update Approved.The
deleteReferencesTo
method update to include thedeleteSearchReferences
call is necessary for the new functionality.
118-118
: Method Update Approved.The
deleteReferencesTo
method update to set the total count includingtotalSearchAssetCount
is necessary for the new functionality.
527-549
: New Method Approved.The new method
deleteSearchReferences
is necessary for the new functionality.
557-638
: New Method Approved.The new method
getAssetsReferencingUrn
is necessary for the new functionality.
641-669
: New Method Approved.The new method
deleteSearchReferencesForAsset
is necessary for the new functionality.
677-682
: New Method Approved.The new method
getAspectsToUpdate
is necessary for the new functionality.
690-696
: New Method Approved.The new method
shouldDeleteAssetReferencingUrn
is necessary for the new functionality.
698-708
: New Method Approved.The new method
updateAspectForSearchReference
is necessary for the new functionality.
710-747
: New Method Approved.The new method
updateFormsAspect
is necessary for the new functionality.
749-753
: New Method Approved.The new method
createAuditStamp
is necessary for the new functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- metadata-service/factories/src/main/java/com/linkedin/gms/factory/entity/DeleteEntityServiceFactory.java (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- metadata-service/factories/src/main/java/com/linkedin/gms/factory/entity/DeleteEntityServiceFactory.java
64d4172
into
datahub-project:master
This PR is going to introduce a "placeholder" for how we can start handling cleaning up references to entities that are deleted. This only works for forms, but the methods are set up so that we can start handling other use cases and build this out more later.
We will come back to this and add broader support for cleaning up references, but this is a good start for forms entities.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
EntitySearchService
as a parameter for better modularity and scalability.