-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[add] Add parameter validators and their corresponding tests #3888
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
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.
license-eye has checked 3565 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 1837 | 10 | 1718 | 0 |
Click to see the invalid file list
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/ParamValidatorManagerTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/BooleanParamValidatorTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/HostParamValidatorAdapterTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/HostParamValidatorTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/JsonParamValidatorTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/NumberParamValidatorTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/OptionParamValidatorTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/PasswordParamValidatorTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/component/validator/impl/TextParamValidatorTest.java
- hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/helper/MonitorImExportHelperTest.java
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
...rc/test/java/org/apache/hertzbeat/manager/component/validator/ParamValidatorManagerTest.java
Show resolved
Hide resolved
...st/java/org/apache/hertzbeat/manager/component/validator/impl/BooleanParamValidatorTest.java
Show resolved
Hide resolved
...ava/org/apache/hertzbeat/manager/component/validator/impl/HostParamValidatorAdapterTest.java
Show resolved
Hide resolved
.../test/java/org/apache/hertzbeat/manager/component/validator/impl/HostParamValidatorTest.java
Outdated
Show resolved
Hide resolved
.../test/java/org/apache/hertzbeat/manager/component/validator/impl/JsonParamValidatorTest.java
Show resolved
Hide resolved
...est/java/org/apache/hertzbeat/manager/component/validator/impl/NumberParamValidatorTest.java
Show resolved
Hide resolved
...est/java/org/apache/hertzbeat/manager/component/validator/impl/OptionParamValidatorTest.java
Show resolved
Hide resolved
...t/java/org/apache/hertzbeat/manager/component/validator/impl/PasswordParamValidatorTest.java
Show resolved
Hide resolved
.../test/java/org/apache/hertzbeat/manager/component/validator/impl/TextParamValidatorTest.java
Show resolved
Hide resolved
...ger/src/test/java/org/apache/hertzbeat/manager/service/helper/MonitorImExportHelperTest.java
Show resolved
Hide resolved
…omponent/validator/ParamValidatorManagerTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/BooleanParamValidatorTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/HostParamValidatorAdapterTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/HostParamValidatorTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/JsonParamValidatorTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
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.
Pull request overview
This pull request refactors the parameter validation logic in HertzBeat by introducing a validator pattern with dedicated parameter validator classes and comprehensive test coverage.
Key Changes:
- Extracts inline validation logic from MonitorServiceImpl into dedicated validator classes using the Strategy pattern
- Creates a ParamValidatorManager to coordinate validation across different parameter types
- Extracts monitor import/export logic into a separate MonitorImExportHelper class for better separation of concerns
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ParamValidator.java | Defines the validator interface for parameter validation |
| ParamValidatorManager.java | Manages and delegates validation to appropriate validators |
| TextParamValidator.java | Validates text and textarea parameter types with length limits |
| PasswordParamValidator.java | Handles password encryption and validation |
| OptionParamValidator.java | Validates radio and checkbox options against defined choices |
| NumberParamValidator.java | Validates numeric parameters and range constraints |
| JsonParamValidator.java | Validates JSON format for metrics-field and key-value types |
| HostParamValidatorAdapter.java | Delegates host validation to existing common validator |
| BooleanParamValidator.java | Validates boolean parameter values |
| ArrayParamValidator.java | Validates array parameters and strips brackets |
| MonitorImExportHelper.java | Extracts import/export logic from MonitorServiceImpl for better modularity |
| MonitorServiceImpl.java | Refactored to use ParamValidatorManager and MonitorImExportHelper, removing 100+ lines of inline validation code |
| MonitorServiceTest.java | Updated to mock ParamValidatorManager and MonitorImExportHelper |
| MonitorImExportHelperTest.java | Tests for the new import/export helper class |
| *ParamValidatorTest.java (multiple) | Comprehensive unit tests for each validator implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ |
Copilot
AI
Dec 4, 2025
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.
This test references a HostParamValidator class that does not exist in the main source code. The only host validator implementation is HostParamValidatorAdapter. Either create the HostParamValidator class or update this test to use HostParamValidatorAdapter instead.
.../test/java/org/apache/hertzbeat/manager/component/validator/impl/HostParamValidatorTest.java
Outdated
Show resolved
Hide resolved
...test/java/org/apache/hertzbeat/manager/component/validator/impl/ArrayParamValidatorTest.java
Outdated
Show resolved
Hide resolved
...beat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java
Show resolved
Hide resolved
...c/main/java/org/apache/hertzbeat/manager/component/validator/impl/BooleanParamValidator.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hertzbeat/manager/component/validator/impl/ArrayParamValidator.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hertzbeat/manager/component/validator/impl/ArrayParamValidator.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/org/apache/hertzbeat/manager/component/validator/ParamValidatorManager.java
Outdated
Show resolved
Hide resolved
…omponent/validator/impl/NumberParamValidatorTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/OptionParamValidatorTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/PasswordParamValidatorTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…ervice/helper/MonitorImExportHelperTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/HostParamValidatorTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/ArrayParamValidatorTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/TextParamValidatorTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/BooleanParamValidator.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…ervice/impl/MonitorServiceImpl.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/ArrayParamValidator.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/ParamValidatorManager.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
…omponent/validator/impl/ArrayParamValidator.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: aias00 <liuhongyu@apache.org>
What's changed?
Checklist
Add or update API