pb33f / libopenapi

libopenapi is a fully featured, high performance OpenAPI 3.1, 3.0 and Swagger parser, library, validator and toolkit for golang applications.
https://pb33f.io/libopenapi/
Other
383 stars 50 forks source link

Missing Line and Column Information in model.ChangeContext for Enum Changes. #231

Closed Amitrei closed 6 months ago

Amitrei commented 6 months ago

Hi there, We are currently exploring the integration of your remarkable tool into our company\'s workflow. One of the essential functionalities we require involves comparing two OpenAPI documents and presenting the changes within our custom UI, akin to the OpenAPI diff report.

The issue

The current challenge we face is related to the model.ChangeContext object when modifying an enum. Unfortunately, this object does not include information about the new/original line and column, which is crucial for visualizing changes on our UI platform. The issue becomes evident in the following test:

func testEnumChange() { 
    left := `openapi: 3.0
components:
  schemas:
    OK:
      enum: [a,b,c]`

    right := `openapi: 3.0
components:
  schemas:
    OK:
      enum: [a,b,foo]`

    originalDoc, err := libopenapi.NewDocument([]byte(left))
    if err != nil {
        fmt.Println("Failed creating new document from OAS content due to: ", err)
    }
    updatedDoc, err := libopenapi.NewDocument([]byte(right))
    if err != nil {
        fmt.Println("Failed creating new document from OAS content due to: ", err)
    }
    compareResults, _ := libopenapi.CompareDocuments(originalDoc, updatedDoc)

    for _, change := range compareResults.GetAllChanges() {
        fmt.Println("Detected the following change:")

        if change.Original != "" {
            fmt.Printf("property: %v \n original value: %v \n line: %v \n column: %v \n", change.Property, change.Original, *change.Context.OriginalLine, *change.Context.OriginalColumn)
        }

        if change.New != "" {
            fmt.Printf("property: %v \n new value: %v \n line: %v \n column: %v \n", change.Property, change.New, *change.Context.NewLine, *change.Context.NewColumn)
        }

    }

}

the results are:

Detected the following change:
property: enum 
 new value: foo 
 line: 0 
 column: 0 
Detected the following change:
property: enum 
 original value: c 
 line: 0 
 column: 0 

The cause

This problem stems from the encoding of the yaml.Node object in the toString method, used here.

The toString implementation:

func toString(v any) string {
    if y, ok := v.(*yaml.Node); ok {
        _ = y.Encode(&v)
    }

    return fmt.Sprint(v)
}

The Line and Column fields, representing the node position in the decoded YAML text, are not considered during the encoding process.

when simplyfing the toString implementation to:

func toString(v any) string {
    return fmt.Sprint(v)
}

the results from the test above are now fixed:

Detected the following change:
property: enum 
 new value: foo 
 line: 5 
 column: 18 
Detected the following change:
property: enum 
 original value: c 
 line: 5 
 column: 18 

Best regards, Amit

daveshanley commented 6 months ago

Hi Amit,

Please feel free to submit a PR to fix update this code, it looks like you've done all the hard work already, why not submit it and join the community?

daveshanley commented 6 months ago

Fixed in v0.14.6

https://github.com/pb33f/libopenapi/releases/tag/v0.14.6