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

Document the public API #392

Closed
erayd opened this issue Mar 17, 2017 · 8 comments
Closed

Document the public API #392

erayd opened this issue Mar 17, 2017 · 8 comments

Comments

@erayd
Copy link
Contributor

erayd commented Mar 17, 2017

What

The public API needs to be documented to make it clear what is intended for user access (i.e. cannot be broken without a major version bump), and what is internal to the library.

Why

So that we know what can and cannot be altered without breaking backwards-compatibility when making internal changes to the library or adding new features.

@erayd
Copy link
Contributor Author

erayd commented Mar 23, 2017

Public API (6.0.0)

As of 6.0.0, I think that the public API of json-schema comprises the following. Anything not in this list should be able to be changed in any manner without bumping the major version number, provided it does not impact the behavior of the stuff listed here in a non-backwards-compatible manner.

Methods

Validator

  • Validator::__construct - create a validator instance with a custom factory
  • Validator::validate - main entry point for validation
  • Validator::check - BC alias of validate (pre-5.0.0), does not pass $value by reference
  • Validator::coerce - BC alias of validate (pre-5.0.0), sets CHECK_MODE_COERCE_TYPES
  • Validator::getErrors - get array of validation errors
  • Validator::numErrors - get number of validation errors
  • Validator::getErrorMask - get the error categories
  • Validator::isValid - get overall validation status
  • Validator::reset - reset validation status

Factory

  • Factory::__construct - create a Factory instance with custom SchemaStorage, UriRetriever or $checkmode
  • Factory::setConfig - overwrite $checkMode
  • Factory::getConfig - get $checkMode or a filtered subset
  • Factory::addConfig - add options to $checkMode
  • Factory::removeConfig - remove options from $checkMode
  • Factory::getSchemaStorage - get the current SchemaStorage instance
  • Factory::getUriRetriever - get the current UriRetriever instance

SchemaStorage & SchemaStorageInterface

  • SchemaStorage::__construct - create a SchemaStorage instance with custom UriRetriever or UriResolver
  • SchemaStorage::addSchema - add a schema to the storage instance by identifyng URI
  • SchemaStorage::getSchema - get a schema from the storage instance by identifying URI

UriResolver & UriResolverInterface

  • UriResolver::resolve - resolve a URI to absolute

UriRetriever & UriRetrieverInterface

  • UriRetriever::retrieve - retrieve & return a decoded JSON schema object

Class Constants

  • Validator::SCHEMA_MEDIA_TYPE - official JSON schema media type
  • Validator::ERROR_* - error categories (bitmask)
  • Constraint::CHECK_MODE_* - validation options (bitmask)

Class Properties

  • Validator::$factory - dependency injection, state & instance management

Data Formats

  • The array data returned by Validator::getErrors()

Exceptions

  • InvalidArgumentException
  • InvalidConfigException
  • InvalidSchemaException
  • InvalidSchemaMediaTypeException
  • InvalidSourceUriException
  • JsonDecodingException
  • ResourceNotFoundException
  • RuntimeException
  • UnresolvableJsonPointerException
  • UriResolverException
  • ValidationException

Please let me know if I have missed anything, or if anything in the list above needs changing. I'm fully aware that there are public methods which are not in this list - I deliberately omitted stuff I felt should not be considered part of the public API, so if you think I've omitted something that should be included, sing out!

@erayd
Copy link
Contributor Author

erayd commented Mar 23, 2017

Also, if this happens, then there will also be methods on that class which need to be considered part of the public API.

@curry684
Copy link

Some food for thought: http://fabien.potencier.org/pragmatism-over-theory-protected-vs-private.html

Fabien makes some good points about protected methods also implicitly being part of the public API of a project. It's more pronounced in a project like Symfony of course, but the core points stand that for example BaseConstraint::$factory is also part of the public API as someone might inherit their own validator from Validator, giving them protected access to that property. That would cause breakage if it were changed in a minor/patch update.

If you wish to restrict this it could be a good idea to make the property itself private and have a protected getter.

@erayd
Copy link
Contributor Author

erayd commented Mar 23, 2017

@curry684 Unfortunately, the way this project is architected means that there will always be methods that are not part of the public API, which are declared as public or protected. No way to avoid that other than significant refactoring of the library that will break a lot of people's workflows.

From your link:

By the way, public methods does not define the public API. Sometimes, you need to declare a method as public because you need to access it from another class in your library; but that does not mean that you want your users to actually call it directly. What do you do then? Symfony and Flow3 have decided to mark the "real" public API with the @api tag. All classes, methods, and properties tagged with @api are public in the sense that we guarantee their stability over time: their name, signature, and behavior won't change for any minor version of the library. That's a strong commitment; one that I like much better than any strict enforcement done on the language level.

This is the kind of thing I'm going for here - to have a well-documented public API, and thereby avoid the confusion that may arise from relying solely on the accessibility to determine whether or not something is part of the API (which won't work for this library in its current architecture).

...for example BaseConstraint::$factory is also part of the public API as someone might inherit their own validator from Validator, giving them protected access to that property.

This is a very good point - I'll need to go through and see which properties need to available for inheritance, and which do not. I have no objection in principle to certain protected properties being considered part of the public API, but I would like to document them as such.

@erayd
Copy link
Contributor Author

erayd commented Mar 23, 2017

Although, adding a getter for Validator::$factory does sound like an increasingly good idea, and would be consistent with how other classes are managing their own injectable dependencies. I will probably do this. Can't make it private though; that would break things.

@curry684
Copy link

curry684 commented Mar 23, 2017

That was my main intent with giving the food for thought: you are both working towards a new major version and documenting the API. That is the best time, if any, to evaluate all extension points that ever happened to get created, intentionally or not. In 6.0 it's perfectly acceptable to change a property from protected to private and give it a protected getter instead. Internal breakage should be zero with a proper refactoring tool.

@bighappyface
Copy link
Collaborator

bighappyface commented May 16, 2017

This proposal works for me as a first phase for documentation

@DannyvdSluijs
Copy link
Collaborator

Closing this issue for now since there isn't any ask for it and we are very bound for time/contribution.

@DannyvdSluijs DannyvdSluijs closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2024
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

4 participants