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

OpenAPI and GraphQL ValidatorHandlers conflict on config file names #252

Closed
logi opened this issue Aug 10, 2018 · 6 comments
Closed

OpenAPI and GraphQL ValidatorHandlers conflict on config file names #252

logi opened this issue Aug 10, 2018 · 6 comments

Comments

@logi
Copy link
Collaborator

logi commented Aug 10, 2018

Both com.networknt.openapi.ValidatorHandler and com.networknt.graphql.validator.ValidatorHandler have CONFIG_NAME = "validator" so if both are loaded in the same JVM they'll both run

static ValidatorConfig config = (ValidatorConfig)Config.getInstance().getJsonObjectConfig(CONFIG_NAME, ValidatorConfig.class);

but with different ValidatorConfig classes imported, so the second one to be loaded fails with

Exception in thread "main" java.lang.ExceptionInInitializerError
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:264)
        at com.networknt.handler.Handler.splitClassAndName(Handler.java:262)
        at com.networknt.handler.Handler.initHandlers(Handler.java:214)
        at com.networknt.handler.Handler.<clinit>(Handler.java:38)
        at com.networknt.server.Server.start(Server.java:136)
        at com.networknt.server.Server.main(Server.java:111)
Caused by: java.lang.ClassCastException: com.networknt.graphql.validator.ValidatorConfig cannot be cast to com.networknt.openapi.ValidatorConfig
        at com.networknt.openapi.ValidatorHandler.<clinit>(ValidatorHandler.java:49)
        ... 7 more

Currently the content of the two files seems to be compatible, but it would not be surprising if that would also diverge.

@stevehu
Copy link
Contributor

stevehu commented Aug 10, 2018

Yes. It is great that you bring it up. We never thought we are going to use two or more different frameworks in the same service instance before. At this moment, there are more uses for the light-rest-4j than light-graphql-4j. For this release, maybe we should change the validator.yml for graphql to graphql-validator.yml and change the hybrid framework validator config to hybrid-validator.yml. When we move to 1.6.x release I think we should change the light-rest-4j to rest-validator.yml @ddobrin @logi @NicholasAzar What do you think?

@logi
Copy link
Collaborator Author

logi commented Aug 10, 2018

Sounds reasonable. Perhaps even renaming the REST config as well but with a fallback to the current name and log a deprecation warning?

@stevehu
Copy link
Contributor

stevehu commented Aug 11, 2018

That is a good idea. Thanks. I will open issues to address them all together.

@stevehu
Copy link
Contributor

stevehu commented Aug 12, 2018

@logi This has been implemented in light-rest-4j, light-graphql-4j and light-hybrid-4j. Please take a look see if the implementation is expected. Thanks.

@logi
Copy link
Collaborator Author

logi commented Aug 13, 2018

About this comment:

# A generic light-4j framework validator configuration. If multiple frameworks are used in the
# same server instance and they have different configurations. You can use openapi-validator.yml

If multiple frameworks are used in the same server instance, even if the files are identical then there will be a ClassCastException as the identical config file names are used as the key in the config lookup map and then cast to different config classes.

This means that if two frameworks are used in the same server, since the compiled-in config files are still all called validator.yml you must create multiple foo-validator.yml files, even if they are identical to the compiled-in files.

@stevehu
Copy link
Contributor

stevehu commented Aug 13, 2018

Cool. I am not aware of this until now. Thanks for the clarification. I will update the comments accordingly.

@stevehu stevehu closed this as completed Aug 16, 2018
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

No branches or pull requests

2 participants