Skip to content

Check if string map param is literal before returning the value. - #133

Merged
jun-he merged 3 commits into
mainfrom
jun/enhance-merge-param
Aug 15, 2025
Merged

Check if string map param is literal before returning the value.#133
jun-he merged 3 commits into
mainfrom
jun/enhance-merge-param

Conversation

@jun-he

@jun-he jun-he commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes (Please run ./gradlew build --write-locks to refresh dependencies)
  • Other (please describe): improve the param merges

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Check if string map param is literal before turning the value.

@jun-he
jun-he requested a review from Copilot August 12, 2025 20:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves parameter merging by adding validation to ensure string map parameters are literal values before processing. The change prevents merging attempts on SEL (expression-based) string map parameters, which would be invalid operations.

  • Added validation to check if string map parameters are literal before accessing their values
  • Enhanced test coverage to verify the new validation behavior
  • Minor code style improvements including diamond operator usage and string formatting cleanup

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
ParamsMergeHelper.java Added literal validation check for string map parameters before value extraction
ParamsMergeHelperTest.java Added test case for the new validation and applied minor code style improvements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

paramDef.isLiteral(),
"param [%s] definition exp=[%s] is not a literal",
key,
paramDef.getExpression());

Copilot AI Aug 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message format uses 'exp=[%s]' but 'paramDef.getExpression()' may return null for literal parameters. Consider using a more descriptive message that handles null expressions or checks if the parameter has an expression before including it.

Suggested change
paramDef.getExpression());
paramDef.getExpression() != null ? paramDef.getExpression() : "N/A");

Copilot uses AI. Check for mistakes.
@jun-he jun-he changed the title Check if string map param is literal before turning the value. Check if string map param is literal before returning the value. Aug 12, 2025
@jun-he
jun-he merged commit 3303c37 into main Aug 15, 2025
1 check passed
@Netflix Netflix deleted a comment from Ferluffy Aug 23, 2025
@jun-he
jun-he deleted the jun/enhance-merge-param branch August 23, 2025 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants