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

Support for schemas with OpenAPI 3 validators #40

Closed
wants to merge 1 commit into from

Conversation

jlavelle
Copy link
Member

The new Validator associated type and FleeceValidator superclass constraint on the Fleece class allow extensions to validation beyond lifting Haskell functions.

I added a class called OpenApi3Validator to the json-fleece-openapi3 package that captures most of the OpenAPI 3 validations. Fleece instances can implement the OpenApi3Validator class on their associated Validator type to gain access to static information associated with the validations.

The intent is that downstream users will leverage the additional information to generate richer OpenAPI schemas based on their Fleece schemas.

There is a new constraint called FleeceOpenApi3 that includes the OpenApi3Validator constraint on the Fleece instance's Validator. It is exported by the json-fleece-openapi3 package, along with associated schemas that use the constraint.

The schemas serve as a drop-in replacement for those in Fleece.Core.Schemas. They use the methods on OpenApi3Validator to implement their validations.

I didn't add support for the pattern validation because I don't have good sense of what regex library to use.

The new `Validator` associated type and `FleeceValidator` superclass
constraint on the `Fleece` class allow extensions to validation beyond
lifting Haskell functions.

I added a class called `OpenApi3Validator` to the `json-fleece-openapi3`
package that captures most of the OpenAPI 3 validations. `Fleece`
instances can implement the `OpenApi3Validator` class on their
associated `Validator` type to gain access to static information
associated with the validations.

The intent is that downstream users will leverage the additional
information to generate richer OpenAPI schemas based on their Fleece
schemas.

There is a new constraint called `FleeceOpenApi3` that includes the
`OpenApi3Validator` constraint on the `Fleece` instance's `Validator`.
It is exported by the `json-fleece-openapi3` package, along with
associated schemas that use the constraint.

The schemas serve as a drop-in replacement for those in
`Fleece.Core.Schemas`. They use the methods on `OpenApi3Validator` to
implement their validations.

I didn't add support for the `pattern` validation because I don't have
good sense of what regex library to use.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file didn't change, I just moved it.

@@ -41,7 +41,8 @@ library
, base >=4.7 && <5
, bytestring ==0.11.*
, containers ==0.6.*
, json-fleece-core ==0.7.*
, json-fleece-core ==0.8.*
, json-fleece-openapi3 ==0.5.*
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love how this added this dependency to a bunch of the packages. The instances need to live somewhere, so the alternatives were reversing this and adding a json-fleece-aeson (and all of the others) dependency to json-fleece-openapi3, or putting the OpenApi3Validator class in json-fleece-core. Neither of those felt right. We could use a cabal flag and the CPP to selectively add the dependency and instance instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The swagger2 and openapi3 packages aren't maintained very actively, so I think it would be nice if we could avoid depending on them unconditionally.

Copy link
Member Author

@jlavelle jlavelle Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. It's actually pretty terrible to have openapi3 as a transitive dependency of json-fleece-aeson (and everything else) now that I think about it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it would be nice to support using fleece without needing to depend on openapi. How bad does adding json-fleece-aeson and others as dependencies to json-fleece-openapi3 seem? Ideally someone in theory could extend fleece with their own custom validation classes in a separate package right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How bad does adding json-fleece-aeson and others as dependencies to json-fleece-openapi3 seem?

Given that json-fleece-aeson is a dependency of the generated code I don't think it would be bad. That being said, the class I originally called OpenApi3Validator should really just be something like JsonSchemaValidator, since it doesn't do anything OpenAPI-specific, and probably in a separate package.

Comment on lines +53 to +56
type FleeceOpenApi3 schema =
( FC.Fleece schema
, OpenApi3Validator (FC.Validator schema)
)
Copy link
Member Author

@jlavelle jlavelle Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A consequence of implementing things this way is that users have to replace their Fleece constraints with FleeceOpenApi3 constraints if they want to use the validator. An alternative would be to move the OpenApi3Validator class to json-fleece-core and export a constraint like this that's just called Fleece, potentially renaming the existing class. In other words, we'd be saying that OpenAPI 3 validations are the default. I didn't do that initially because it seemed too opinionated, but maybe it's a better approach (it would also solve the annoyance I mentioned above about adding the json-fleece-openapi3 dependency everywhere).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's only if they want to use the validator that seems fine. I don't think we'd want to demand OpenApi3 (or any) participation for all of Fleece, though, if that's what you mean by the alternative.

Tangentially it appears that we only have one API client that isn't using OpenAPI 3 -- tai software (they have two versions of their API and for both we are using swagger codegen). I messages them to see if they have an OpenAPI 3 spec.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constraint OpenApi3Validator (FC.Validator schema) means that the validators you write can use the methods on the OpenApi3Validator class, which are only related to OpenAPI 3 in that they represent the validators supported by the OpenAPI 3 spec, i.e. you don't have to be using OpenAPI 3 anywhere else to use the validators.

So in that sense, the alternative of placing the OpenApi3Validator class and the constraint above in json-fleece-core doesn't demand OpenAPI 3 participation, it just enables you to write validators in terms of those supported by OpenAPI 3.

Given @ysangkok's point above about the transitive openapi3 dependency I introduced into all of the fleece packages, it would actually be less demanding of participation to move these modules into json-fleece-core.

The idea would be to:

  1. rename the Fleece class to something like FleeceSchema
  2. export a constraint like the above called Fleece

We could potentially rename OpenApi3Validator to something else as well, though I'm not sure that there another name that makes more sense. Maybe the methods should just be part of the FleeceValidator class instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually there are good reasons to avoid making them part of FleeceValidator, e.g. type and format values as Text don't necessarily have interpretations outside of the context of OpenAPI 3.

@jlavelle
Copy link
Member Author

I'm going to close this and make a new PR with just the changes to json-fleece-core for now.

@jlavelle jlavelle closed this Aug 16, 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

Successfully merging this pull request may close these issues.

4 participants