Export OpenAPI definitions from service examples; affects [dynamic endpoint static]

[chris48s]

Refs #7982

This PR changes lots of things, and also nothing 🙃

One of the things we need to do in order to migrate the frontend to docusaurus-openapi is to produce an OpenAPI spec for each service/endpoint we expose.

This PR does a few main things:

• Allows service classes to optionally define an openApi property directly
• Adds some code that makes a decent (but imperfect in some cases) attempt to derive one from the existing examples array
• Adds a script which we can use to export OpenAPI definitions that can then be consumed by a frontend that can read them
• Adds OpenAPI definitions to a handful of services. I've started off by adding these to the dynamic badges, endpoint badge and static badges because in the new frontend I want to document these the same as any other endpoint but in the existing frontend they have custom "documentation" (if you can call it that) and no examples, so we can do this without having any impact on the existing site.

In the interests of keeping changesets reasonably small and easy-ish to review, I'm not doing anything about adding that frontend here - just generating the OpenAPI specs. Also, merging this PR will not change anything about the existing frontend code. Merging this would give us the next piece of the jigsaw to start working on this but we can deploy this without affecting anything.

Next steps from here (may not be exhaustive) are something like:

• work on switching out the existing gatsby-based frontend for docusaurus-openapi using the best-effort OpenAPI definitions generated from examples[]
• once we switch the frontends, (auto-)convert all the existing examples arrays on service classes to openApi objects
• look for ways we can reduce the duplication between the service definition and OpenAPI spec
• make openApi required/validated
• delete all examples properties + remove any validation for it
• tweak and improve from there (e.g: fixing required query params, removing unnecessary duplicates for query param variants, adding param descriptions) - this could be a gradual process
• delete all the code for bludgeoning examples arrays into an OpenAPI spec (this will clean things up a lot)
• update contributing docs for adding new services to reflect new frontend

The commits in this PR may not be especially helpful. I kind of meandered about a bit. It might be best to just look at the whole diff to review.

Hopefully this all makes sense as a direction.

There are a couple of failing service tests which are not related to this PR. They are also failing on the daily-tests repo. I'm going to ignore them for now.

[github-actions]

	Warnings
⚠️	This PR modified service code for dynamic but not its test code. That's okay so long as it's refactoring existing code.
⚠️	This PR modified service code for endpoint but not its test code. That's okay so long as it's refactoring existing code.
⚠️	This PR modified service code for static-badge but not its test code. That's okay so long as it's refactoring existing code.

	Messages
📖	✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against a153c1d

[chris48s]

There's some stuff we just can't really populate from the existing data structure. Rather than try to extend it, move to allowing services to declare openapi path directly.

[calebcartwright]

Could you elaborate on this for me? I'm not confident I understand the logic and comments of the if/else block and the conditionality of required

[chris48s]

I know you resolved this comment, but I think it is worth explaining this one in more detail.

Basically, in our existing examples array that we attach to service classes, we don't say if a param is required or optional.
For path (or route) params, this doesn't matter. All path params are required in OpenAPI.
For query params, this does matter because in OpenAPI they can be required or optional, and we do have a few services where there are query params that are required at the schema level.
For the moment I'm just setting them all to optional, which is right in most cases but there will be a handful of cases where we are documenting a required query param as optional in the frontend but then requiring it at the schema level. These will need improving later, but the way I want to do this is by moving to declaring an openApi spec object directly on the service classes, rather than extending the existing examples object (which already requires some bashing with a hammer to get it into the right format).

[chris48s]

TODO: Moar/better docs, typedef

[chris48s]

In most cases, this is duplicating information which exists in queryParamSchema. It would be nice to infer one from the other, but I think I'm not going to get bogged down in that just yet.

[chris48s]

Merged in master. Service tests now passing

[calebcartwright]

Probably not worth it, at least not as part of this PR, but intuition is that we should either try to extract this from the route variable that's assigned a value above at the createRoute call site, or perhaps we should just make the route explicit inline here; It just gives me pause that we've got a hardcoded value here two lines down from where this is set dynamically in a function call, even though I know we're not changing the actual route values ever

[chris48s]

Agreed. This comes a few steps further on in the migration plan I laid out.
This is exactly the sort of thing I have in mind when I say "look for ways we can reduce the duplication between the service definition and OpenAPI spec" in the top post - there is definitely information we are specifying twice.
I think I want to have a slightly more holistic view of it all first to arrive at the right abstractions.

[calebcartwright]

The commits in this PR may not be especially helpful. I kind of meandered about a bit. It might be best to just look at the whole diff to review.

I remember reading this back in March, but I'd definitely forgotten it by the time I got around to reviewing. I left a few inline comments/questions/suggestions, several of which were unnecessary and stemmed from reviewing by commit so I closed them out. A couple of other minor thought comments were left open, but none are blocking, especially given how long this has been open and how it is a blocker itself to other work.

The only concern I have is that it feels like there's a lot of what I'll call "domain specifics" around OpenAPI that are definitely unfamiliar to me, and some non-trivial logic in place to reshape some of our existing structures into those domain specifics. I don't think it's great to only have one person understand those well, but I also don't think that's an adequate enough justification to not do anything.

The frontend needs a refresh, that "domain specifics" problem would exist with any solution, and this solution seems as reasonable as any a path forward

[chris48s]

I remember reading this back in March, but I'd definitely forgotten it by the time I got around to reviewing. I left a few inline comments/questions/suggestions, several of which were unnecessary and stemmed from reviewing by commit so I closed them out

Yes. I think there are a couple of challenges with this PR (and the next one). One of them is that it was one of those PRs where I had a vague idea where I was going, but didn't really know exactly what the endgame looked like until I was into the problem (hence the commits kind of wandered around a bit).

The second one is that there is a tension between

• Attempt to arrive at a perfect solution in one go and boil the ocean in enormous PRs that change everything all at once
• Tackle the problem in smaller chunks (which hopefully makes it easier to review/make changes incrementally), at the expense of introducing some sub-optimal or non-obvious intermediate steps

I'm trying to tackle this in smaller chunks, but the tradeoff of that there are clear improvements that can be made and some of the code I've written here is stop-gap code I hope to delete in the near future. I am also trying to communicate where I envisage we go next with this. I appreciate that it is much clearer to me where we should go next with this, whereas for the person reviewing it you can only really see clearly where I've gone so far.

The only concern I have is that it feels like there's a lot of what I'll call "domain specifics" around OpenAPI that are definitely unfamiliar to me

Yes. This is a good point 👍 Once we drop the examples array and every service class exports an openApi object, two of the things we really need to do are:

a) More clearly document the subset of OpenAPI that we (and contributors) need to declare on a service class and how that subset specifically applies to shields (this is what I'm getting at with the point "update contributing docs for adding new services to reflect new frontend")
b) Implement some automated checks (be that Joi schema, CI checks, whatever) that help to prevent common issues/make it easy to "fall into the pit of success"

While we are still declaring an examples array on the service class for 99% of services and haven't quite nailed down how much of the openapi bit needs to be handwritten vs generated, we're not quite in that place yet.

I think this will also get a lot easier once there is a decent body of examples to crib from too.

In general, I think OpenAPI is probably a good direction of travel though. If you pick a random developer, there is more chance they've some familiarity reading or writing an OpenAPI spec (a widely used standard) than a shields examples array (a thing we made up).

[chris48s]

I'm going to go ahead and merge this and get back into finishing off #9014