Basic skeleton for plugging custom rules in the validation chain. #388

thiyagu06 · 2022-08-15T08:41:23Z

This is the first to attempt improve this validation tool to be more extensible and customisable as discussed in #347. I just created basic skeleton on how we can implement a custom rule in the existing feature. The design is inspired from the custom generator approach. The idea is at very nascent stage, we can discuss further to improve this validator to be more extensible and customisable.

port all existing rules into new design
make CLI, maven and gradle plugin to use the new rule structure with backward compatible way
update README.md to inculde info on how to add custom rules

cli/src/main/java/OperationHasTagRule.java

cli/src/main/java/CheckTagsAreDefinedRule.java

JFCote · 2022-08-15T17:03:07Z

Hi @thiyagu06 ! Thanks for this PR. Looks very interesting. I'm currently on vacation and will look closely when I get back. Thanks. In the meantime, you can check for the Lift recommandation. Also, don't worry about the build failing. It's because of Sonarqube.

JFCote · 2022-08-21T19:12:16Z

Hi @thiyagu06, please update with latest master. I have removed the sonarqube scan (too much bugs/missing features). It should build without issues now.

JFCote

I have put a lot of comments, hope that makes sense. Very good start, just need some adjustment I think. And this could lead to something very nice!

JFCote · 2022-08-21T19:18:46Z

cli/src/main/java/rules/CheckTagsAreDefinedRule.java

+/**
+ * This rule will check whether tags are defined in the rule section
+ */
+public class CheckTagsAreDefinedRule extends OpenAPIRule {


I love the idea of custom rules! Why did you put them in the CLI modules? I think they should go in the lib module so anyone could use them (cli, java, maven, gradle, etc...). But maybe there was a reason. Let me know!

Yes.. you're right. it should not be there in the cli module. I just put it there to validate my design such that loadAndExecuteRules() method is able to find all the rules available in the class path.

cli/src/main/java/rules/CheckTagsAreDefinedRule.java

lib/src/main/java/org/openapitools/openapistylevalidator/ErrorAggregator.java

lib/src/main/java/org/openapitools/openapistylevalidator/UserDefinedRuleExecutor.java

...ain/java/org/openapitools/openapistylevalidator/custom/CheckOperationTagsAreDefinedRule.java

lib/src/main/java/org/openapitools/openapistylevalidator/custom/OpenAPIRule.java

thiyagu06 · 2022-08-24T13:13:23Z

Hi @JFCote ,I hope I answered most of your queries. I will try to find sometime during this week to refine more on this approach.

lib/src/main/java/org/openapitools/openapistylevalidator/rules/ContactInfoRule.java

lib/src/main/java/org/openapitools/openapistylevalidator/RulesManager.java

…es into new design. I

…iple style errors from the specific rule. see HeaderNamingRule

sonatype-lift · 2022-08-28T14:17:18Z

lib/src/main/java/org/openapitools/openapistylevalidator/ValidatorParameters.java


        private final String designation;

-        NamingConvention(String appelation) {
+        private final Predicate<String> predicate;


ImmutableEnumChecker: enums should be immutable: 'NamingConvention' has field 'predicate' of type 'java.util.function.Predicate<java.lang.String>', the declaration of type 'java.util.function.Predicate<java.lang.String>' is not annotated with @com.google.errorprone.annotations.Immutable

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2022-08-28T14:17:20Z

lib/src/main/java/org/openapitools/openapistylevalidator/rules/ParameterNamingRule.java

+public class ParameterNamingRule implements Rule {
+
+    public static final String PARAMETER_NAMING = "parameter naming";
+    private final RuleParameterProvider ruleParameterProvider;


💬 2 similar findings have been found in this PR

UnusedVariable: The field 'ruleParameterProvider' is never read.

Expand here to view all instances of this finding

File Path Line Number

lib/src/main/java/org/openapitools/openapistylevalidator/rules/PathNamingRule.java 13

lib/src/main/java/org/openapitools/openapistylevalidator/rules/PropertyNamingRule.java 13

Visit the Lift Web Console to find more details in your report.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2022-08-28T14:17:21Z

lib/src/main/java/org/openapitools/openapistylevalidator/naming/PatternPredicates.java

+    Predicate<String> camelCaseMatcher =
+            (input) -> notEmpty.test(input) && input.matches("[a-z][a-z\\d]*(?:[A-Z\\d]+[a-z\\d]*)*");
+    Predicate<String> hyphenCaseMatcher =
+            (input) -> notEmpty.test(input) && input.matches("[a-z][a-z\\d]*(?:-[a-z\\d]+)*");


💬 4 similar findings have been found in this PR

REDOS: The regular expression "[a-z][a-z\d](?:-[a-z\d]+)" is vulnerable to a denial of service attack (ReDOS)

Expand here to view all instances of this finding

File Path Line Number

lib/src/main/java/org/openapitools/openapistylevalidator/naming/PatternPredicates.java 10

lib/src/main/java/org/openapitools/openapistylevalidator/naming/PatternPredicates.java 12

lib/src/main/java/org/openapitools/openapistylevalidator/naming/PatternPredicates.java 14

lib/src/main/java/org/openapitools/openapistylevalidator/naming/PatternPredicates.java 18

Visit the Lift Web Console to find more details in your report.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

thiyagu06 · 2022-08-28T14:25:56Z

Hi @JFCote , If you feel the PR is huge and trying to do too much in a single PR. I can split into multiple logical unit and raise another PR once we agreed upon the new design. You can take a look at an sample here custom rules to see my thoughts in actions.

JFCote · 2022-08-31T11:37:58Z

Hey @thiyagu06 , I'm back from vacation and will try to look at this in the upcoming days.
Thanks

JFCote · 2022-09-06T02:05:50Z

Hi @thiyagu06 ! Sorry for the delay! I really like the design and would like that we start working towards that. I'm thinking this could take a lot of PRs and love the idea that we go step by step, assuring that nothing is broken along the way. So I guess this PR will be closed and you will start fresh?

thiyagu06 · 2022-09-06T13:43:54Z

@JFCote , Thanks for the comments. I will think about more on how to split up the work without breaking the existing flow in coming weeks and create small logical PRs 😄 .

Basic skeleton idea for plugging custom rules in the validation chain.

f6b528c

thiyagu06 changed the title ~~Basic skeleton idea for plugging custom rules in the validation chain.~~ Basic skeleton for plugging custom rules in the validation chain. Aug 15, 2022

sonatype-lift bot reviewed Aug 15, 2022

View reviewed changes

cli/src/main/java/OperationHasTagRule.java Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Aug 15, 2022

View reviewed changes

cli/src/main/java/CheckTagsAreDefinedRule.java Outdated Show resolved Hide resolved

move custom rules from default packages to rules package.

be19552

JFCote requested changes Aug 21, 2022

View reviewed changes

thiyagu06 force-pushed the custom-rules branch from b09f682 to 712e863 Compare August 27, 2022 16:34

thiyagu06 marked this pull request as ready for review August 27, 2022 16:35

sonatype-lift bot reviewed Aug 27, 2022

View reviewed changes

lib/src/main/java/org/openapitools/openapistylevalidator/rules/ContactInfoRule.java Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Aug 27, 2022

View reviewed changes

lib/src/main/java/org/openapitools/openapistylevalidator/RulesManager.java Outdated Show resolved Hide resolved

thiyagu06 added 8 commits August 28, 2022 19:40

Merge branch 'master' into custom-rules

0a0c6c5

remove unused methods.

4b02fb8

Added new api module to support custom rule and ported 3 existing rul…

bbb2dc3

…es into new design. I

fixed sonatype lift recommendation

8b8bb87

added naming convention rule design and implemented header naming rules

a3505e0

changed return type of rule api to list as we required to return mult…

52adb42

…iple style errors from the specific rule. see HeaderNamingRule

changed return type of rule api to list as we required to return mult…

f9b3d2b

…iple style errors from the specific rule. see HeaderNamingRule

Remove dependencies of validatorParameters class in Rule manager

87503a8

thiyagu06 force-pushed the custom-rules branch from fae0b8a to 87503a8 Compare August 28, 2022 14:12

sonatype-lift bot reviewed Aug 28, 2022

View reviewed changes

thiyagu06 mentioned this pull request Sep 8, 2022

added new api module for creating custom rules and moved error classes to new module #400

Merged

thiyagu06 closed this Sep 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic skeleton for plugging custom rules in the validation chain. #388

Basic skeleton for plugging custom rules in the validation chain. #388

thiyagu06 commented Aug 15, 2022 •

edited

Loading

JFCote commented Aug 15, 2022

JFCote commented Aug 21, 2022

JFCote left a comment

JFCote Aug 21, 2022

thiyagu06 Aug 24, 2022 •

edited

Loading

thiyagu06 commented Aug 24, 2022 •

edited

Loading

sonatype-lift bot Aug 28, 2022

sonatype-lift bot Aug 28, 2022

sonatype-lift bot Aug 28, 2022

thiyagu06 commented Aug 28, 2022

JFCote commented Aug 31, 2022

JFCote commented Sep 6, 2022

thiyagu06 commented Sep 6, 2022

Basic skeleton for plugging custom rules in the validation chain. #388

Basic skeleton for plugging custom rules in the validation chain. #388

Conversation

thiyagu06 commented Aug 15, 2022 • edited Loading

JFCote commented Aug 15, 2022

JFCote commented Aug 21, 2022

JFCote left a comment

Choose a reason for hiding this comment

JFCote Aug 21, 2022

Choose a reason for hiding this comment

thiyagu06 Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

thiyagu06 commented Aug 24, 2022 • edited Loading

sonatype-lift bot Aug 28, 2022

Choose a reason for hiding this comment

sonatype-lift bot Aug 28, 2022

Choose a reason for hiding this comment

sonatype-lift bot Aug 28, 2022

Choose a reason for hiding this comment

thiyagu06 commented Aug 28, 2022

JFCote commented Aug 31, 2022

JFCote commented Sep 6, 2022

thiyagu06 commented Sep 6, 2022

thiyagu06 commented Aug 15, 2022 •

edited

Loading

thiyagu06 Aug 24, 2022 •

edited

Loading

thiyagu06 commented Aug 24, 2022 •

edited

Loading