Skip to content
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

Header definition clashing with naming conventions #148

Open
acabarbaye opened this issue Jan 15, 2021 · 13 comments · Fixed by #153
Open

Header definition clashing with naming conventions #148

acabarbaye opened this issue Jan 15, 2021 · 13 comments · Fixed by #153
Assignees
Milestone

Comments

@acabarbaye
Copy link

Describe the bug
When describing header parameters which do not follow any naming convention such as Accept-Version or if-none-match, I get naming convention errors for PATH and Properties

To Reproduce
e.g.

 parameters:
    acceptVersionParam:
      name: Accept-Version
      in: header

results in
*ERROR* in path GET /... 'Accept-Version' -> parameter should be in camelCase
*ERROR* in path GET / 'Accept-Version' -> parameter should be in camelCase

Expected behavior
No error raised for headers as we have no control over their naming

@JFCote
Copy link
Member

JFCote commented Jan 15, 2021

Hi @acabarbaye ,

There will be a new parameter for this, it has been added last week with this issue: #135

We should release a new version with this improvement somewhere next week. In the meantime, you can compile the code of the project in the master.

Let me know if this new feature is ok for your use case.

@JFCote
Copy link
Member

JFCote commented Jan 19, 2021

@acabarbaye It has been released in version 1.5. Please let us know if it's working for you and we will close this issue :)

@acabarbaye
Copy link
Author

@JFCote : has version 1.5 made it to maven central or Jcenter?

   > Could not find org.openapitools.openapistylevalidator:openapi-style-validator-cli:1.5.
     Searched in the following locations:
       - https://jcenter.bintray.com/org/openapitools/openapistylevalidator/openapi-style-validator-cli/1.5/openapi-style-validator-cli-1.5.pom
       - https://jcenter.bintray.com/org/openapitools/openapistylevalidator/openapi-style-validator-cli/1.5/openapi-style-validator-cli-1.5-all.jar
       - https://repo.maven.apache.org/maven2/org/openapitools/openapistylevalidator/openapi-style-validator-cli/1.5/openapi-style-validator-cli-1.5.pom
       - https://repo.maven.apache.org/maven2/org/openapitools/openapistylevalidator/openapi-style-validator-cli/1.5/openapi-style-validator-cli-1.5-all.jar

Just to be sure I understand the new feature, for my case, I should define the following options?

{... 
  "validateNaming": true,
  "ignoreHeaderXNaming": true,
  "headerNamingStrategyConvention": HyphenCase,
}

@JFCote
Copy link
Member

JFCote commented Jan 21, 2021

@acabarbaye Oh! It seems the release didn't make its way to Maven. I will check with @jmini who did the release. Will update here when it's ready.

As for your problem, I now see that it will not fix your issue because the naming of your header is not the same. At first glance, I thought that it was the same thing as #135

What you need would be an option to completely ignore the naming of headers. I will think about it. Maybe a new "naming strategy convention" that will be Anything?

@JFCote
Copy link
Member

JFCote commented Jan 22, 2021

@acabarbaye There was a problem during the release, the 1.5 was lost. It has been generated again and it's there! https://repo1.maven.org/maven2/org/openapitools/openapistylevalidator/openapi-style-validator-lib/1.5/

@JFCote JFCote self-assigned this Jan 22, 2021
@JFCote JFCote added this to the v1.6.0 milestone Jan 22, 2021
JFCote added a commit that referenced this issue Jan 22, 2021
… Also, finally removed the deprecated stuff since we have released at least 3 version since the deprecated warnings.
@acabarbaye
Copy link
Author

@acabarbaye There was a problem during the release, the 1.5 was lost. It has been generated again and it's there! https://repo1.maven.org/maven2/org/openapitools/openapistylevalidator/openapi-style-validator-lib/1.5/

Perfect.
I will give it another try but I think your suggestion of having a Anything option would make a lot of sense

@JFCote
Copy link
Member

JFCote commented Jan 25, 2021

@acabarbaye I created a PR last friday. It just needs to be reviewed and the feature should work.

JFCote added a commit that referenced this issue Feb 24, 2021
* Fix intellij meta data and configuration. WIP

* fix #148: Add a new `anyCase` naming convention to ignore everything. Also, finally removed the deprecated stuff since we have released at least 3 version since the deprecated warnings.

* Fix from code review
@gervaisb
Copy link
Contributor

gervaisb commented Mar 29, 2021

I have documented some official headers like Accept-Encoding and If-None-Match. However they did not pass any naming convention. I tried AnyCase but received a NullPointerException.

Anyway, Can't we exclude official headers from the naming convention validation or include a naming convention that match the official "Hyphen-Upper-Case" ? And/or maybe ensure that official headers match the official naming convention ?

Edit; The NullPointerException is gone with v1.6.

@JFCote
Copy link
Member

JFCote commented Mar 30, 2021

@gervaisb Just to be sure, is it ok now or do you still have problems with the AnyCase naming convention?

@gervaisb
Copy link
Contributor

Well the issue with AnyCase is that it accept anything. I still feel that there is a missing HyphenUpperCase style to validate headers like Accept-Encoding.

gervaisb added a commit to gervaisb/openapi-style-validator that referenced this issue Mar 30, 2021
@gervaisb
Copy link
Contributor

gervaisb commented Jul 30, 2021

Hello. Can we reopen that issue ?

I have tried the version 1.7 but the "Hyphen-Upper-Case" cause a NPE.

Exception in thread "main" java.lang.NullPointerException
at org.openapitools.openapistylevalidator.NamingValidator.isNamingValid(NamingValidator.java:82)
at org.openapitools.openapistylevalidator.OpenApiSpecStyleValidator.validateNaming(OpenApiSpecStyleValidator.java:207)
at org.openapitools.openapistylevalidator.OpenApiSpecStyleValidator.validate(OpenApiSpecStyleValidator.java:35)
at org.openapitools.openapistylevalidator.cli.Main.validate(Main.java:62)
at org.openapitools.openapistylevalidator.cli.Main.validateAndPrint(Main.java:47)
at org.openapitools.openapistylevalidator.cli.Main.main(Main.java:36)

I would like to:

  • Patch OpenApiSpecStyleValidator.validateNaming(..) so that it accepts the "Hyphen-Upper-Case" naming convention.
  • Update README.md to document that naming convention.
  • Manage this NullPointerException to provide a detailed error message with all supported naming convention.

@JFCote JFCote reopened this Jul 30, 2021
@JFCote
Copy link
Member

JFCote commented Jul 30, 2021

@gervaisb Issue reopened!

@JFCote JFCote modified the milestones: v1.6.0, v1.8.0 Sep 24, 2021
@gervaisb
Copy link
Contributor

gervaisb commented Oct 5, 2021

I have tried the version 1.7 but the "Hyphen-Upper-Case" cause a NPE.

I do not know what happened with 1.7, but I have made another attempt right now (Oct. 5 2021) and it seems that HyphenUpperCase is a supported naming convention in version 1.7. 🎉

Anyway, I still have to update the README and manage the NullPointer. However, the NullPointerException occurs because Gson ignores unknown values for enums. So we have to either validate that there are no null naming conventions in ValidatorParameters (once parsed), but then we do not have the original values to provide a complete error message. Another solution would be to validate them against the enum values before the parsing (this is my favourite solution).

Whatever the choice, the Main class does not have a validation mechanism for the parameters. A quick solution would be to validate into the Main class, right after optionManager.getOptionalValidatorParametersOrDefault (L62) and throws IllegalArgumentException that will be caught aside of the ParseException (L42). But this is not ideal and there should be a more complete solution to validate the parameters/options.

Do you have any advice on where and how that validation should be implemented?

gervaisb added a commit to gervaisb/openapi-style-validator that referenced this issue Oct 8, 2021
Add this convention to the lists of naming conventions and describe the capability
to validate the headers.

Relates to OpenAPITools#148
JFCote added a commit that referenced this issue Oct 8, 2021
* Fix 178, should not fail when there are no 'example' for a component

* Update lib/src/main/java/org/openapitools/openapistylevalidator/OpenApiSpecStyleValidator.java

little reformat

* chore(doc): document the Hyphen-Upper-Case naming convention

Add this convention to the lists of naming conventions and describe the capability
to validate the headers.

Relates to #148

Co-authored-by: Jean-François Côté <jcote@stingray.com>
gervaisb added a commit to gervaisb/openapi-style-validator that referenced this issue Oct 8, 2021
The cli will now validate the naming conventions and report them trough an
IllegalArgumentException with a detailed message instead of a
NullPointerException.

Relates to OpenAPITools#148

This commit will also fix a typo in `fixConventionRenaming`: "depre[a]cated"
JFCote added a commit that referenced this issue Oct 8, 2021
* Fix 178, should not fail when there are no 'example' for a component

* Update lib/src/main/java/org/openapitools/openapistylevalidator/OpenApiSpecStyleValidator.java

little reformat

* feat(cli): validate naming conventions

The cli will now validate the naming conventions and report them trough an
IllegalArgumentException with a detailed message instead of a
NullPointerException.

Relates to #148

This commit will also fix a typo in `fixConventionRenaming`: "depre[a]cated"

Co-authored-by: Jean-François Côté <jcote@stingray.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants