Skip to content

add a SEL function to check if a param exists and improve the SEL doc #118

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

Merged
merged 1 commit into from
Jun 17, 2025

Conversation

jun-he
Copy link
Collaborator

@jun-he jun-he commented Jun 17, 2025

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):

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

Changes in this PR

add a SEL function to check if a param exists and improve the SEL doc

@jun-he jun-he requested a review from Copilot June 17, 2025 05:34
Copy link

@Copilot Copilot AI left a comment

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 introduces a new SEL function containsKey to check if a parameter exists, adds corresponding tests, and updates the SEL documentation.

  • Added containsKey handling in SelParams.call
  • Added unit tests for containsKey in SelParamsTest
  • Expanded docs with containsKey in class-function.md

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
netflix-sel/src/main/java/com/netflix/sel/type/SelParams.java Added containsKey branch to call method
netflix-sel/src/test/java/com/netflix/sel/type/SelParamsTest.java New tests for containsKey behavior
netflix-sel/docs/lang-guide/class-function.md Documentation for containsKey function
Comments suppressed due to low confidence (3)

netflix-sel/src/test/java/com/netflix/sel/type/SelParamsTest.java:61

  • Add a test case for calling containsKey with an incorrect number of arguments (e.g. zero args) to verify it throws UnsupportedOperationException, mirroring the existing testInvalidCallGet.
}

netflix-sel/docs/lang-guide/class-function.md:195

  • [nitpick] Grammar: change “currently only support string type” to “currently only supports string types” for clarity.
return params.getFromSignalOrDefault('signal1', 'param1', 'value1');    // returns parameter param1's value from signal1, if there is any exception, then return the default value (currently only support string type)

netflix-sel/src/main/java/com/netflix/sel/type/SelParams.java:50

  • Indent this else if and its closing brace consistently with the surrounding if/else blocks to match the project’s formatting conventions (run spotlessApply to fix).
} else if (args.length == 1 && "containsKey".equals(methodName)) {

@jun-he jun-he merged commit a1246ed into main Jun 17, 2025
1 check passed
@jun-he jun-he deleted the jun/improve-sel branch June 17, 2025 05:42
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.

1 participant