Library Improvement

[danopz]

Motivation: I wanted to build a PSR-7 compliant Swagger Validation and searched for some libraries I can use for this. So I found this library and looked a bit through the code and made a simple test yet. I wanted to try to write an adapter to use PSR-7 requests for validation but this is nearly impossible/very hard with the current implementation. So I made a list what could improve the validator for the future. The following list was just hacked together in a few minutes and without much thinking about it, so it could miss something or could contain pointless stuff and is just my first opinion.

• get rid of monolithic classes like SwaggerValidator\Common\Context
• use DI together with interface type hints instead of implementations
  • get rid of static calls
  • get rid of singletons
• use PSR-3 for (real) logging stuff
• use 'use statements' instead of FQNS in code
• allow constructing of the definition with object/array - don't force to use a path to the json file
• add classes(interfaces) for request and response to not directly use global php variables
  • we could add implementations that use the globals
  • we even could use PSR-7, since this is the new standard way to handle requests and responses in php frameworks
• do we still need the phar and the autoloader?

What are your thoughts @nabbar ?

[nabbar]

Hi, some of your feature will be present in the release 1.4. But first understand that this capabilities will still stay on the code :

• the full capabilities of override all the class (except collection)
• a context class that allow to having the all information about a event (error, succes, wrong openapi schema, ...)
• lib driven by the schema and not a lib who use the schema

The logger PSR 3 compliant / data collector PSR 7 compliant are sure plannied to be in milestones 1.4 In add, I work on technical documentation about this validator. This is a major less in this current library

The phar / autoloader was here to allow to be implment in old php application who's not compatible with composer version restriction. I think the phar will stay but the autoloader will use the composer loader.

In the next 1.4, a major thing will change : the code will be rewrite in native PHP 7. For use in PHP release before PHP 7, users must stay in release 1.3.x version.

[nabbar]

Other thinks : globals are not used except some magic globals to having a default method to load data, but this will extract in a default data collector dedicated class who's must be override by user when implement the libs.

[danopz]

the full capabilities of override all the class (except collection)

Of course you theoretically can extend the existing classes and override the methods, but that doesn't help at all. Especially the current Context class is nearly impossible to override because of this big codebase and the codebase is inherits from, but the thing is all the validation stuff requires to add an instance of the this exact implementation instead of clean interfaces. Also there are some static calls in the implementation to other implementations which everyone needs to override for own implementations.

a context class that allow to having the all information about a event (error, succes, wrong openapi schema, ...)

For me those are no events or some kind of context, they are just validation states, happened on checking request/response against the provided schema.

lib driven by the schema and not a lib who use the schema

I don't really understand this one.

data collector PSR 7 compliant are sure plannied to be in milestones 1.4

Sounds good.

but the autoloader will use the composer loader

How should it use it? If we go all in for the Composer autoloader we dont need the custom one at all.

In the next 1.4, a major thing will change : the code will be rewrite in native PHP 7.

This is not a valid SemVer change. If you rewrite the major codebase or, like this one, drop PHP5 there must be a new major instead of new minor.

this will extract in a default data collector dedicated class who's must be override by user when implement the libs

Would be ok for me if there is not 'a class we must override' but an interface with a default implementation for $_POST, $_GET etc and can be implemented and injected.

In fact I thought more of a rewrite of the library for flexibility and future proof instead of some adapters. Do you have any progress on 1.4 which we can review a bit?

[nabbar]

Of course you theoretically can extend the existing classes and override the methods, but that doesn't help at all. Especially the current Context class is nearly impossible to override because of this big codebase and the codebase is inherits from, but the thing is all the validation stuff requires to add an instance of the this exact implementation instead of clean interfaces. Also there are some static calls in the implementation to other implementations which everyone needs to override for own implementations.

I agree that the Context class is horrible to extant or just tu use and it is planned to be refactored to be more easiest to use. I planned to explose it into dedicated and smallest and smallest classes.

How should it use it? If we go all in for the Composer autoloader we dont need the custom one at all.

That's I want to saying

This is not a valid SemVer change. If you rewrite the major codebase or, like this one, drop PHP5 there must be a new major instead of new minor.

You're right sorry for the mistake

Would be ok for me if there is not 'a class we must override' but an interface with a default implementation for $_POST, $_GET etc and can be implemented and injected.

Ok, I can write the classes for test and just use an interface on the lib

[danopz]

What did you have in mind - adding functionality, refactor the library or start new? I think it would be good for this lib if the new version will be started from scratch. Sure some small parts could stay, but Imho there should be a restart build on Interfaces and open standards.

What do you think? Of course you don't have to share my opinions.