Add support for different bundling modes #350
Conversation
bundler/bundler.go
Outdated
| bundledComponents := map[string]*index.ReferenceNode{ | ||
| target | ||
| } | ||
|
|
||
| childRefs := targetIndex.ExtractRefs(refTarget.Node, refTarget.ParentNode, make([]string, 0), 0, false, "") | ||
|
|
||
| for _, childRef := range childRefs { | ||
| childRefTarget := targetMappedReferences[childRef.FullDefinition] | ||
| childComponents, err := bundleRefTarget(childRef, childRefTarget, opts) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
This part here is incomplete because I didn't figure out what to do with the components collected from child references.
The original code manipulated the Content field of the ref's yaml.Node and then just triggered the Render method on the document.
What I figure we need to do in this PR is to properly manipulate the root v3.Document by adding new nodes to the components section. I guess this could be done by manipulating the corresponding yaml.Nodes.
However, I started wondering whether the Document itself or the high/low models should be updated. I started wondering whether there is/should be an API for manipulating the v3.Document. I ended up looking at the various ordered maps that are being used under the hood, but eventually ran out of time.
I'm thinking it might be possible to update the yaml.Nodes and call RenderAndReload() after manipulating the YAML. This should work, if one ensures that all refs in the root document are already referring to components in the same document.
@daveshanley, any suggestions how to proceed here?
There was a problem hiding this comment.
There are two ways to do this.
- Go down low (the hard, long, but the right way).
This would be to do what you said, to manipulate the model at the low end and build up the document manually by creating high level objects from low level ones, and then adding them to the correct component location.
- Build high (the simpler, faster way, questionable on correctness)
Just build out the new components using the high level, add them as references (https://pb33f.io/libopenapi/modifying/#using-references) and then render out the doc.
Why is this questionable? well, it means if we provide this as an API, the only way to populate the low level
is to RenderAndReload and it would push another run of the model again. Will it cost anything, some CPU and memory - but not much to be honest. Is it right? not exactly - but will work as long as the use is documented.
There was a problem hiding this comment.
So the answer to your question is, choose the path you can invest in. If you have the time and the stomach, number 1. It will make for a better API, design and a less costly implementation.
If you do not have time to spare (like any of us) then I suggestion option 2. It will work, it's not ideal, but it's also not wrong either - just not crisp and not performant.
|
If this gets done this would be amazing. I think this is what redocly-cli does. Basically preserves the refs as local refs but moves them to their correct location in a single file. Bonus is that it'll work correctly for recursive items. Even better if there's support for unique naming of refs as well |
|
Had very little time to work on this lately, but still want to do it. |
|
FYI ... I have a PR open in the libvalidator project where I was 'nudged' to follow the "Options pattern" model which enables a more fluent style to configuration options. My initial submission followed a very similar "options model" to the one you've got here ;-) |
|
I love when ideas converge! |
|
@JemDay thanks. Yeah, familiar with that. |
felixjung
force-pushed
the
bundling-modes
branch
from
865fdb0 to
80d86bd
Compare
felixjung
force-pushed
the
bundling-modes
branch
from
78ac884 to
77f5090
Compare
| bundledComponents := make(map[string]*index.ReferenceNode) | ||
| if err := bundleRefTarget(sequenced, mappedReference, bundledComponents, opts); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Made a tiny bit of progress today. The idea is that bundledComponents now contains all the yaml.Nodes which are to be bundled into the root spec, using their Definition (i.e. #/components/schemas/MySchema) as keys.
What I'm stuck with now, is that I don't know how to actually use these nodes. I figured the "right" way, you mentioned in your previous comments, would be to manipulate the low model by adding to the components there. I just can't make sense of the types used there. See my next comment.
| key := low.KeyReference[string]{ | ||
| Value: defParts[3], | ||
| KeyNode: &yaml.Node{ | ||
| Kind: yaml.ScalarNode, | ||
| Style: yaml.TaggedStyle, | ||
| Tag: "!!str", | ||
| Value: defParts[3], | ||
| }, | ||
| } | ||
| value := low.ValueReference[*base.SchemaProxy]{ | ||
| Reference: low.Reference{}, | ||
| Value: &base.SchemaProxy{ | ||
| Reference: low.Reference{}, | ||
| NodeMap: &low.NodeMap{Nodes: &sync.Map{}}, | ||
| }, | ||
| ValueNode: &yaml.Node{}, | ||
| } | ||
| components.Value.Schemas.Value.Set(key, value) |
There was a problem hiding this comment.
It would appear, I need to put together the key and value so I can set them in the ordered map for Schemas. However, with all the Value, ValueNode, Reference etc. fields, I really don't understand what I'd be doing here. 😅 Can you once again point me in the right direction, please?
The current bundling approach in libopenapi automatically inlines all refs in files other than the root file. The
BundleInlineRefsoption indatamodel.DocumentConfigurationappears to only affect the root file. When set totrue, refs in the root file are inlined. When set tofalse, refs within the root file, i.e. to thecomponentssection, are kept where they are, while refs made to other files are inlined nevertheless.The goal of this PR is to introduce different bundling modes.
componentssection or other sections (as necessary) in the bundled spec.The bundle mode can be configured in a new
BundlerOptionsstruct passed to the bundling functions of the bundler package.