[#9536] feat(core): Add event listener for function operations#10499
[#9536] feat(core): Add event listener for function operations#10499mchades merged 4 commits intoapache:mainfrom
Conversation
Code Coverage Report
Files
|
|
Hi @laserninja, thanks for your contributions—is this PR ready for review? |
|
I had a question but I'll save it for after the review since it might be redundant |
Add event/listener hooks for function CRUD operations (register, get, alter, drop, list). This follows the same pattern as existing entity event dispatchers (Table, Model, etc.). New files: - FunctionInfo: read-only function info for event listeners - FunctionEvent/FunctionFailureEvent/FunctionPreEvent: base event classes - Register/Get/Alter/Drop/ListFunction Event/PreEvent/FailureEvent classes - FunctionEventDispatcher: decorates FunctionDispatcher with event dispatching - TestFunctionEvent: unit tests for all function events Modified files: - OperationType: added REGISTER/GET/ALTER/DROP/LIST_FUNCTION - GravitinoEnv: wired FunctionEventDispatcher into dispatcher chain Closes apache#9536
a07f67b to
180c0b0
Compare
There was a problem hiding this comment.
Pull request overview
Adds event/listener hooks around function catalog operations by introducing function-specific event types and wiring a FunctionEventDispatcher into the core dispatcher chain, aligning function CRUD/list operations with the existing event-dispatching patterns used for other entities (e.g., tables/models).
Changes:
- Introduced
FunctionEventDispatcherto emit pre/success/failure events for function operations and delegate to the underlyingFunctionDispatcher. - Added function event/info types (
FunctionInfo,FunctionEvent/FunctionPreEvent/FunctionFailureEvent, and operation-specific event classes) plus newOperationTypeentries. - Added unit tests validating function event dispatch for success/failure paths.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/test/java/org/apache/gravitino/listener/api/event/TestFunctionEvent.java | Adds unit tests for function operation events (success/failure). |
| core/src/main/java/org/apache/gravitino/listener/FunctionEventDispatcher.java | Decorator that dispatches function pre/post/failure events around function operations. |
| core/src/main/java/org/apache/gravitino/listener/api/info/FunctionInfo.java | Read-only function info DTO used in events. |
| core/src/main/java/org/apache/gravitino/listener/api/event/RegisterFunctionPreEvent.java | Pre-event for register function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/RegisterFunctionFailureEvent.java | Failure-event for register function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/RegisterFunctionEvent.java | Success-event for register function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/OperationType.java | Adds function operation types. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListFunctionPreEvent.java | Pre-event for listing functions. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListFunctionFailureEvent.java | Failure-event for listing functions. |
| core/src/main/java/org/apache/gravitino/listener/api/event/ListFunctionEvent.java | Success-event for listing functions. |
| core/src/main/java/org/apache/gravitino/listener/api/event/GetFunctionPreEvent.java | Pre-event for get function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/GetFunctionFailureEvent.java | Failure-event for get function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/GetFunctionEvent.java | Success-event for get function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/FunctionPreEvent.java | Base pre-event type for function operations. |
| core/src/main/java/org/apache/gravitino/listener/api/event/FunctionFailureEvent.java | Base failure-event type for function operations. |
| core/src/main/java/org/apache/gravitino/listener/api/event/FunctionEvent.java | Base success-event type for function operations. |
| core/src/main/java/org/apache/gravitino/listener/api/event/DropFunctionPreEvent.java | Pre-event for drop function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/DropFunctionFailureEvent.java | Failure-event for drop function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/DropFunctionEvent.java | Success-event for drop function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/AlterFunctionPreEvent.java | Pre-event for alter function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/AlterFunctionFailureEvent.java | Failure-event for alter function. |
| core/src/main/java/org/apache/gravitino/listener/api/event/AlterFunctionEvent.java | Success-event for alter function. |
| core/src/main/java/org/apache/gravitino/GravitinoEnv.java | Wires FunctionEventDispatcher into the function dispatcher chain. |
core/src/main/java/org/apache/gravitino/listener/FunctionEventDispatcher.java
Show resolved
Hide resolved
core/src/test/java/org/apache/gravitino/listener/api/event/TestFunctionEvent.java
Show resolved
Hide resolved
core/src/main/java/org/apache/gravitino/listener/api/event/function/AlterFunctionEvent.java
Show resolved
Hide resolved
… and add LIST_FUNCTION_INFOS
core/src/main/java/org/apache/gravitino/listener/FunctionEventDispatcher.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/gravitino/listener/api/event/function/AlterFunctionPreEvent.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/gravitino/listener/api/info/FunctionInfo.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/gravitino/listener/api/event/function/AlterFunctionEvent.java
Outdated
Show resolved
Hide resolved
mchades
left a comment
There was a problem hiding this comment.
you should also update the doc
gravitino/docs/gravitino-server-config.md
Line 169 in bbb14ad
…ate docs - Extract PrincipalUtils.getCurrentUserName() to local variable 'user' in all FunctionEventDispatcher methods for consistency with ModelEventDispatcher - Use Arrays.copyOf() instead of .clone() in AlterFunctionEvent and AlterFunctionFailureEvent constructors - Add defensive copy in AlterFunctionPreEvent constructor - Return defensive copies from functionChanges() getters in AlterFunctionEvent, AlterFunctionFailureEvent, AlterFunctionPreEvent - Return defensive copy from definitions() getter in FunctionInfo - Add function event entries to gravitino-server-config.md docs
|
Updated the doc. Thanks for the review! |
mchades
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your contributions!
I'll merge it into the main branch after the CI passes
|
BTW, please update the PR description to match the format. |
|
Done, thanks |
…pache#10499) ### What changes were proposed in this pull request? Add event/listener hooks for function CRUD operations (register, get, alter, drop, list). This follows the same pattern as existing entity event dispatchers (Table, Model, etc.). ### Why are the changes needed? New files: - FunctionInfo: read-only function info for event listeners - FunctionEvent/FunctionFailureEvent/FunctionPreEvent: base event classes - Register/Get/Alter/Drop/ListFunction Event/PreEvent/FailureEvent classes - FunctionEventDispatcher: decorates FunctionDispatcher with event dispatching - TestFunctionEvent: unit tests for all function events Modified files: - OperationType: added REGISTER/GET/ALTER/DROP/LIST_FUNCTION - GravitinoEnv: wired FunctionEventDispatcher into dispatcher chain Closes apache#9536 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? All tests passed
What changes were proposed in this pull request?
Add event/listener hooks for function CRUD operations (register, get, alter, drop, list). This follows the same pattern as existing entity event dispatchers (Table, Model, etc.).
Why are the changes needed?
New files:
Modified files:
Closes #9536
Does this PR introduce any user-facing change?
No
How was this patch tested?
All tests passed