-
Notifications
You must be signed in to change notification settings - Fork 138
Fix BuiltInFunctionResolver #1812
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
Conversation
…text - Replace direct method invocation calls in unit tests with a new `invokeFunction` utility method for cleaner code and better readability. - Update the `SpELExchangeEvaluationContext` to use `addMethodResolver` instead of `setMethodResolvers` for adding built-in functions. - Adjust `ReflectiveMethodHandler` to provide a clearer separation between retrieving methods and invoking them. - Remove unused `jsonPath` method from `BuiltInFunctions`. - Enhance the `BuiltInFunctionResolver` to utilize the new method handling approach in resolving function calls.
Caution Review failedThe pull request is closed. WalkthroughThe changes refactor the handling of function resolution and invocation in the SpEL (Spring Expression Language) integration. The process of retrieving and invoking built-in functions is now separated into distinct steps, with new methods introduced to handle retrieval and invocation individually. Error handling is adjusted to fail early if a method cannot be retrieved. Corresponding updates are made to the registration of method resolvers and to the unit tests, which are adapted to the new two-step function handling process and expanded to cover additional scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test
participant Resolver as BuiltInFunctionResolver
participant Handler as ReflectiveMethodHandler
participant Method as Java Method
Test->>Resolver: resolve(functionName, argumentTypes)
Resolver->>Handler: retrieveFunction(functionName, argumentTypes)
alt method found
Handler-->>Resolver: Method
Resolver-->>Test: MethodExecutor lambda
Test->>MethodExecutor: execute(context, arguments)
MethodExecutor->>Handler: invokeFunction(Method, context, arguments)
Handler->>Method: invoke(arguments)
Method-->>Handler: result
Handler-->>MethodExecutor: TypedValue
MethodExecutor-->>Test: TypedValue
else method not found
Handler-->>Resolver: null
Resolver-->>Test: null
end
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Add a new method `jsonPath` to the `BuiltInFunctions` class for parsing JSON based on a given path. Enhance `BuiltInFunctionsTest` with a test case to validate the functionality, ensuring it returns correct values for existing and non-existing paths.
Remove duplicate test for built-in function and consolidate to a single test case that validates the evaluation of the `weight` function.
core/src/main/java/com/predic8/membrane/core/lang/spel/functions/BuiltInFunctionResolver.java
Show resolved
Hide resolved
core/src/test/java/com/predic8/membrane/core/lang/spel/SpELExchangeExpressionTest.java
Show resolved
Hide resolved
…t for list contains functionality - Update BuiltInFunctionResolver to clarify null return is required by the interface specification. - Add a new unit test to verify the list contains functionality in SpELExchangeExpressionTest.
…signature Remove unnecessary exceptions from retrieveFunction method in ReflectiveMethodHandler. Update BuiltInFunctionResolver to handle fewer exceptions during method resolution and improve clarity with added documentation.
Summary by CodeRabbit
Bug Fixes
Tests