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

Add reference to originating locations in YAML specs - step 1 #1007

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

reuvenharrison
Copy link
Contributor

@reuvenharrison reuvenharrison commented Aug 25, 2024

This PR adds the ability to correlate OpenAPI components with the location in the originating YAML spec.
Here's a summary of the changes required to achieve this:

  1. Modify yaml.v3 to add line and column numbers of each YAML element to the unmarshaled interfaces (see https://github.com/oasdiff/yaml3)
  2. Modify invopop/yaml with a new function: UnmarshalWithOrigin (see https://github.com/oasdiff/yaml)
  3. Modify this repo to add an Origin struct to OpenAPI components. This requires changes to the UnmarshalJSON functions and, in some cases, replacing native types, like map[string]string, by dedicated types that include the original native type and an Origin member.

This PR addresses issue #986

Follow up tasks to be addressed by future PRs:

  • Add origin to more elements, namely Components and some types that are unmarshalled with unmarshalStringMapP or unmarshalStringMap
  • Merge changes in yaml.v3 and invopop/yaml back to original repos and remove replace directives from go.mod
  • Enhance origin to include end line and column and YAML comments as proposed by @fenollp

@reuvenharrison reuvenharrison marked this pull request as ready for review September 21, 2024 07:56
@reuvenharrison reuvenharrison changed the title Correlate OpenAPI components with their location in the originating YAML spec Add reference to originating locations in YAML specs - step 1 Sep 21, 2024
Copy link
Collaborator

@fenollp fenollp left a comment

Choose a reason for hiding this comment

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

A few minor edits otherwise LGTM

Comment on lines +115 to +167
// UnmarshalJSON sets Callbacks to a copy of data.
func (callbacks *Callbacks) UnmarshalJSON(data []byte) (err error) {
*callbacks, _, err = unmarshalStringMapP[CallbackRef](data)
return
}

// UnmarshalJSON sets Examples to a copy of data.
func (examples *Examples) UnmarshalJSON(data []byte) (err error) {
*examples, _, err = unmarshalStringMapP[ExampleRef](data)
return
}

// UnmarshalJSON sets Headers to a copy of data.
func (headers *Headers) UnmarshalJSON(data []byte) (err error) {
*headers, _, err = unmarshalStringMapP[HeaderRef](data)
return
}

// UnmarshalJSON sets Links to a copy of data.
func (links *Links) UnmarshalJSON(data []byte) (err error) {
*links, _, err = unmarshalStringMapP[LinkRef](data)
return
}

// UnmarshalJSON sets ParametersMap to a copy of data.
func (parametersMap *ParametersMap) UnmarshalJSON(data []byte) (err error) {
*parametersMap, _, err = unmarshalStringMapP[ParameterRef](data)
return
}

// UnmarshalJSON sets RequestBodies to a copy of data.
func (requestBodies *RequestBodies) UnmarshalJSON(data []byte) (err error) {
*requestBodies, _, err = unmarshalStringMapP[RequestBodyRef](data)
return
}

// UnmarshalJSON sets ResponseBodies to a copy of data.
func (responseBodies *ResponseBodies) UnmarshalJSON(data []byte) (err error) {
*responseBodies, _, err = unmarshalStringMapP[ResponseRef](data)
return
}

// UnmarshalJSON sets Schemas to a copy of data.
func (schemas *Schemas) UnmarshalJSON(data []byte) (err error) {
*schemas, _, err = unmarshalStringMapP[SchemaRef](data)
return
}

// UnmarshalJSON sets SecuritySchemes to a copy of data.
func (securitySchemes *SecuritySchemes) UnmarshalJSON(data []byte) (err error) {
*securitySchemes, _, err = unmarshalStringMapP[SecuritySchemeRef](data)
return
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please define these in their respective files

Comment on lines -14 to +15
Mapping map[string]string `json:"mapping,omitempty" yaml:"mapping,omitempty"`
PropertyName string `json:"propertyName" yaml:"propertyName"` // required
Mapping StringMap `json:"mapping,omitempty" yaml:"mapping,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move the introduction of the StringMap type to another PR, with a breaking change note to README if applicable.

AuthorizationURL string `json:"authorizationUrl,omitempty" yaml:"authorizationUrl,omitempty"`
TokenURL string `json:"tokenUrl,omitempty" yaml:"tokenUrl,omitempty"`
RefreshURL string `json:"refreshUrl,omitempty" yaml:"refreshUrl,omitempty"`
Scopes StringMap `json:"scopes" yaml:"scopes"` // required
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

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.

2 participants