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.

Sign up for GitHub

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 #609

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Add reference to originating locations in YAML specs #609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2b56755
proof of concept
reuvenharrison Aug 23, 2024
7058596
update deps
reuvenharrison Sep 21, 2024
e7f5761
add origin to ApiChange
reuvenharrison Sep 25, 2024
2f2c175
use kin-openapi
reuvenharrison Nov 24, 2024
baa4893
indexes are 1-based
reuvenharrison Nov 24, 2024
8195d3b
getOriginLine, getOriginCol
reuvenharrison Nov 24, 2024
9dc7298
coverage
reuvenharrison Nov 24, 2024
0e5acd7
enable docker on branch
reuvenharrison Nov 24, 2024
ac2f9e2
test WithLocation
reuvenharrison Nov 25, 2024
9643eed
fix docs
reuvenharrison Nov 25, 2024
f22314d
add unit tests
reuvenharrison Nov 25, 2024
77143b8
use latest kin-openapi
reuvenharrison Dec 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ on:
push:
branches:
- 'main'
- 'origin'
tags:
- 'v*'
jobs:
Expand Down Expand Up @@ -50,7 +51,7 @@ jobs:
platforms: linux/amd64,linux/arm64
context: ./
file: ./Dockerfile
tags: tufin/oasdiff:${{ github.ref_name }}, tufin/oasdiff:stable, tufin/oasdiff:latest
tags: tufin/oasdiff:${{ github.ref_name }}
labels: ${{ steps.meta.outputs.labels }}
if: github.ref != 'refs/heads/main' && github.event_name != 'pull_request'

Expand Down
33 changes: 33 additions & 0 deletions checker/api_change.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type ApiChange struct {
}

func NewApiChange(id string, config *Config, args []any, comment string, operationsSources *diff.OperationsSourcesMap, operation *openapi3.Operation, method, path string) ApiChange {

return ApiChange{
Id: id,
Level: config.getLogLevel(id),
Expand All @@ -43,7 +44,39 @@ func NewApiChange(id string, config *Config, args []any, comment string, operati
CommonChange: CommonChange{
Attributes: getAttributes(config, operation),
},
SourceLine: getOriginLine(operation),
SourceColumn: getOriginCol(operation),
}
}

func (c ApiChange) WithLocation(origin *openapi3.Origin, field string) ApiChange {
if origin == nil {
return c
}
location, ok := origin.Fields[field]
if !ok {
return c
}

c.SourceLine = location.Line
c.SourceColumn = location.Column
return c
}

func getOriginLine(operation *openapi3.Operation) int {
if operation == nil || operation.Origin == nil {
return 0
}

return operation.Origin.Key.Line
}

func getOriginCol(operation *openapi3.Operation) int {
if operation == nil || operation.Origin == nil {
return 0
}

return operation.Origin.Key.Column
}

func getAttributes(config *Config, operation *openapi3.Operation) map[string]any {
Expand Down
11 changes: 11 additions & 0 deletions checker/api_change_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package checker_test
import (
"testing"

"github.com/getkin/kin-openapi/openapi3"
"github.com/stretchr/testify/require"
"github.com/tufin/oasdiff/checker"
"github.com/tufin/oasdiff/load"
Expand Down Expand Up @@ -83,3 +84,13 @@ func TestApiChange_SourceUrl(t *testing.T) {

require.Equal(t, "", apiChangeSourceFile.GetSourceFile())
}

func TestApiChange_WithLocation(t *testing.T) {
apiChangeSourceFile := apiChange.WithLocation(&openapi3.Origin{
Fields: map[string]openapi3.Location{"field": {
Line: 1,
Column: 2,
}}}, "field")
require.Equal(t, 1, apiChangeSourceFile.SourceLine)
require.Equal(t, 2, apiChangeSourceFile.SourceColumn)
}
2 changes: 1 addition & 1 deletion checker/check_added_required_request_body.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func AddedRequestBodyCheck(diffReport *diff.Diff, operationsSources *diff.Operat
operationItem.Revision,
operation,
path,
))
).WithLocation(operationItem.Revision.RequestBody.Value.Origin, "required"))
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions checker/check_api_deprecation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ func open(file string) (*load.SpecInfo, error) {
return load.NewSpecInfo(openapi3.NewLoader(), load.NewSource(file))
}

func openWithLocation(file string) (*load.SpecInfo, error) {
loader := openapi3.NewLoader()
loader.IncludeOrigin = true
return load.NewSpecInfo(loader, load.NewSource(file))
}

func getDeprecationFile(file string) string {
return fmt.Sprintf("../data/deprecation/%s", file)
}
Expand Down
2 changes: 1 addition & 1 deletion checker/check_request_body_required_value_updated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func RequestBodyRequiredUpdatedCheck(diffReport *diff.Diff, operationsSources *d
operationItem.Revision,
operation,
path,
))
).WithLocation(operationItem.Revision.RequestBody.Value.Origin, "required"))
}
}
return result
Expand Down
61 changes: 42 additions & 19 deletions checker/check_request_body_required_value_updated_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,46 +11,69 @@ import (

// CL: changing request's body to required is breaking
func TestRequestBodyBecameRequired(t *testing.T) {
s1, err := open("../data/checker/request_body_became_required_base.yaml")
s1, err := openWithLocation("../data/checker/request_body_became_required_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_body_became_required_base.yaml")
s2, err := openWithLocation("../data/checker/request_body_became_required_revision.yaml")
require.NoError(t, err)

s2.Spec.Paths.Value("/api/v1.0/groups").Post.RequestBody.Value.Required = true

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.RequestBodyRequiredUpdatedCheck), d, osm)
require.Len(t, errs, 1)
require.Equal(t, checker.ApiChange{
Id: checker.RequestBodyBecameRequiredId,
Level: checker.ERR,
Operation: "POST",
Path: "/api/v1.0/groups",
Source: load.NewSource("../data/checker/request_body_became_required_base.yaml"),
OperationId: "createOneGroup",
Id: checker.RequestBodyBecameRequiredId,
Level: checker.ERR,
Operation: "POST",
Path: "/api/v1.0/groups",
Source: load.NewSource("../data/checker/request_body_became_required_revision.yaml"),
OperationId: "createOneGroup",
SourceLine: 19,
SourceColumn: 9,
}, errs[0])
}

// CL: changing request's body to optional
func TestRequestBodyBecameOptional(t *testing.T) {
s1, err := open("../data/checker/request_body_became_optional_base.yaml")
s1, err := openWithLocation("../data/checker/request_body_became_optional_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_body_became_optional_base.yaml")
s2, err := openWithLocation("../data/checker/request_body_became_optional_revision.yaml")
require.NoError(t, err)

s2.Spec.Paths.Value("/api/v1.0/groups").Post.RequestBody.Value.Required = false
d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestBodyRequiredUpdatedCheck), d, osm, checker.INFO)
require.Len(t, errs, 1)
require.Equal(t, checker.ApiChange{
Id: checker.RequestBodyBecameOptionalId,
Level: checker.INFO,
Operation: "POST",
Path: "/api/v1.0/groups",
Source: load.NewSource("../data/checker/request_body_became_optional_revision.yaml"),
OperationId: "createOneGroup",
SourceLine: 19,
SourceColumn: 9,
}, errs[0])
}

// CL: changing request's body to optional by deletion
func TestRequestBodyBecameOptionalDeleted(t *testing.T) {
s1, err := openWithLocation("../data/checker/request_body_became_optional_base.yaml")
require.NoError(t, err)
s2, err := openWithLocation("../data/checker/request_body_became_optional_deleted.yaml")
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestBodyRequiredUpdatedCheck), d, osm, checker.INFO)
require.Len(t, errs, 1)
require.Equal(t, checker.ApiChange{
Id: checker.RequestBodyBecameOptionalId,
Level: checker.INFO,
Operation: "POST",
Path: "/api/v1.0/groups",
Source: load.NewSource("../data/checker/request_body_became_optional_base.yaml"),
OperationId: "createOneGroup",
Id: checker.RequestBodyBecameOptionalId,
Level: checker.INFO,
Operation: "POST",
Path: "/api/v1.0/groups",
Source: load.NewSource("../data/checker/request_body_became_optional_deleted.yaml"),
OperationId: "createOneGroup",
SourceLine: 9,
SourceColumn: 5,
}, errs[0])
}
59 changes: 59 additions & 0 deletions data/checker/request_body_became_optional_deleted.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
openapi: 3.0.1
info:
title: Tufin
version: "2.0"
servers:
- url: https://localhost:9080
paths:
/api/v1.0/groups:
post:
tags:
- Group
operationId: createOneGroup
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Creates one project.
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: OK
"409":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Conflict
summary: Create One Project
components:
parameters:
groupId:
in: path
name: groupId
required: true
schema:
type: string
schemas:
GroupView:
type: object
properties:
data:
type: object
properties:
created:
type: string
format: date-time
readOnly: true
pattern: "^[a-z]+$"
id:
type: string
readOnly: true
name:
type: string
required:
- name
60 changes: 60 additions & 0 deletions data/checker/request_body_became_optional_revision.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
openapi: 3.0.1
info:
title: Tufin
version: "2.0"
servers:
- url: https://localhost:9080
paths:
/api/v1.0/groups:
post:
tags:
- Group
operationId: createOneGroup
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Creates one project.
required: false
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: OK
"409":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Conflict
summary: Create One Project
components:
parameters:
groupId:
in: path
name: groupId
required: true
schema:
type: string
schemas:
GroupView:
type: object
properties:
data:
type: object
properties:
created:
type: string
format: date-time
readOnly: true
pattern: "^[a-z]+$"
id:
type: string
readOnly: true
name:
type: string
required:
- name
60 changes: 60 additions & 0 deletions data/checker/request_body_became_required_revision.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
openapi: 3.0.1
info:
title: Tufin
version: "2.0"
servers:
- url: https://localhost:9080
paths:
/api/v1.0/groups:
post:
tags:
- Group
operationId: createOneGroup
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Creates one project.
required: true
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: OK
"409":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Conflict
summary: Create One Project
components:
parameters:
groupId:
in: path
name: groupId
required: true
schema:
type: string
schemas:
GroupView:
type: object
properties:
data:
type: object
properties:
created:
type: string
format: date-time
readOnly: true
pattern: "^[a-z]+$"
id:
type: string
readOnly: true
name:
type: string
required:
- name
Loading
Loading