Removing gson and everit dependencies

[fjtirado]

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:
It removed org.json and everit validation dependencies and add json-schema-validator for validation.
See https://github.com/everit-org/json-schema#when-to-use-this-library
Since we are using Jackson for serialization/deserialization of the workflow, we should use https://github.com/networknt/json-schema-validator instead of everit, and hence avoid having dependencies to two different json parsers.

Special notes for reviewers:
This remove unneeded dependencies for poms and slightly change the validator interface (it now returns a Collection of ValidationErrors rather than a List, which easily remove duplicated validation messages)

Additional information (if needed):

[ricardozanini]

Although this is very important, we need to handle the CVEs while they don't release a new version:

https://mvnrepository.com/artifact/com.github.java-json-tools/json-schema-validator/2.2.14

1. Upgrade Guava - CVE-2023-2976 and CVE-2020-8908
2. Upgrade testng-core - CVE-2022-4065

[ricardozanini]

We can instead use this: https://github.com/networknt/json-schema-validator

[fjtirado]

Although this is very important, we need to handle the CVEs while they don't release a new version:

https://mvnrepository.com/artifact/com.github.java-json-tools/json-schema-validator/2.2.14

1. Upgrade Guava - [CVE-2023-2976](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-2976) and [CVE-2020-8908](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-8908)

2. Upgrade testng-core - [CVE-2022-4065](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-4065)

To remove this CVEs, lets use https://github.com/networknt/json-schema-validator, reworking the PR

[tsurdilo]

Will review. Just note that if you add this much refactoring would be nice to add some additional tests for it in addition to existing.

[tsurdilo]

thanks for the updates. i would really like to request some more tests since its a bigger refactor. can we add some positive/negative tests (and not just one please :) )

[fjtirado]

thanks for the updates. i would really like to request some more tests since its a bigger refactor. can we add some positive/negative tests (and not just one please :) )

@tsurdilo Since this PR, despite apparently large, is not adding any new functionality (just keeping the existing one using a library that avoid dependencies that bring vulnerabilities), fortunately (kudos for the original writer ;)), the tests that are related with the change: basically serialization/deserialization and validation, were already there. In fact, they were changed as result of this change because the valitadion error messaged are library dependent.

[tsurdilo]

Not sure i get point of this class. Just maybe have static instance of yaml/json mappers in a util class?

[fjtirado]

This avoid creating a new object mapper instance every time is needed

[tsurdilo]

ok got it. i think since you return just the basic "new" mapper just define it in static field in some util class no need imo
for separate class. if you want to allow users to set specific configuration for the mappers then having it in some separate class like this maybe makes more sense

[fjtirado]

This is done in a separate class to only initialize the mapper object when it is really used. If you put it into an utils class it will get initialized the firts time you use the utilities class, even in the utility method being used is unrelated (imagine an use that never use yaml format)

[tsurdilo]

Same here

[fjtirado]

same, to avoid creating multiple object mapper instances

[tsurdilo]

why is this change needed?

[fjtirado]

the new validation library tries to download the schema from a not existing url (Because wg-serverless-org is fake), hence changing to real location (which is library classpath)

[tsurdilo]

tell me if im wrong you already define the hard-coded path to the schema in code, if so then why here? i think it was by design to put some id like this and not use classpath

[fjtirado]

this id path is used to upload other schemas referred from this schema
So, if we leave wg-.serverless.org, the library will fail to retrieve, any nested shema, for example, start.json.

[tsurdilo]

i understand what you are saying as in you modified the existing validation tests some and they still pass.
what i am saying is would be nice to add more than just the existing 9 and possibly catch issues the current one might be
missing

[fjtirado]

i understand what you are saying as in you modified the existing validation tests some and they still pass. what i am saying is would be nice to add more than just the existing 9 and possibly catch issues the current one might be missing

I added a couple of tests to ensure that workflows with functions and events are properly validated

[tsurdilo]

I think the validation piece (List/Collection + the added imho ambiguous uniqueness constraint) is whats holding up the pr
I dont have issue making the library change, but the pr also adds this other piece.
Would help maybe if you can remove that added uniques constraint and only have in PR the lib change stuff and then we can unblock the actual change this pr i think intends to doand discuss that uniqueness stuff in separate pr so we don't hold up this one?

[ricardozanini]

@tsurdilo I think this PR is ready since the conversation about List or Set has been settled down and the commit reverted back to what it was?