Skip to content

Commit e262f48

Browse files
committed
Address issue #202
This required the use of the RevisionFS to fully solve this properly.
1 parent fd7ea33 commit e262f48

16 files changed

Lines changed: 752 additions & 219 deletions

cmd/engine.go

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ import (
99

1010
"github.com/pb33f/doctor/changerator"
1111
drModel "github.com/pb33f/doctor/model"
12+
v3 "github.com/pb33f/doctor/model/high/v3"
1213
whatChangedModel "github.com/pb33f/libopenapi/what-changed/model"
1314
"github.com/pb33f/openapi-changes/internal/breakingrules"
1415
"github.com/pb33f/openapi-changes/model"
1516
)
1617

17-
1818
// changeratorResult owns the doctor-side resources created for a single comparison.
1919
//
2020
// Callers must call Release() exactly once when they are done reading Changerator,
@@ -82,6 +82,7 @@ func runChangerator(commit *model.Commit, breakingConfig *whatChangedModel.Break
8282
leftDrDoc.Release()
8383
return nil, nil
8484
}
85+
rewriteOutputLocations(ctr, docChanges, commit.DocumentRewriters)
8586

8687
return &changeratorResult{
8788
Changerator: ctr,
@@ -91,6 +92,94 @@ func runChangerator(commit *model.Commit, breakingConfig *whatChangedModel.Break
9192
}, nil
9293
}
9394

95+
func rewriteOutputLocations(ctr *changerator.Changerator, docChanges *whatChangedModel.DocumentChanges, rewriters []model.DocumentPathRewriter) {
96+
if len(rewriters) == 0 {
97+
return
98+
}
99+
rewriteDocumentChangeLocations(docChanges, rewriters)
100+
if ctr != nil {
101+
rewriteChangedNodeLocations(ctr.ChangedNodes, rewriters)
102+
}
103+
}
104+
105+
func rewriteDocumentChangeLocations(docChanges *whatChangedModel.DocumentChanges, rewriters []model.DocumentPathRewriter) {
106+
if docChanges == nil {
107+
return
108+
}
109+
for _, change := range docChanges.GetAllChanges() {
110+
if change == nil || change.Context == nil || change.Context.DocumentLocation == "" {
111+
continue
112+
}
113+
change.Context.DocumentLocation = rewriteDocumentLocation(change.Context.DocumentLocation, rewriters)
114+
}
115+
}
116+
117+
func rewriteChangedNodeLocations(nodes []*v3.Node, rewriters []model.DocumentPathRewriter) {
118+
if len(nodes) == 0 {
119+
return
120+
}
121+
seen := make(map[*v3.Node]struct{}, len(nodes))
122+
var walk func(*v3.Node)
123+
walk = func(node *v3.Node) {
124+
if node == nil {
125+
return
126+
}
127+
if _, ok := seen[node]; ok {
128+
return
129+
}
130+
seen[node] = struct{}{}
131+
132+
if node.Origin != nil {
133+
node.Origin.AbsoluteLocation = rewriteDocumentLocation(node.Origin.AbsoluteLocation, rewriters)
134+
node.Origin.AbsoluteLocationValue = rewriteDocumentLocation(node.Origin.AbsoluteLocationValue, rewriters)
135+
}
136+
node.OriginLocation = rewriteDocumentLocation(node.OriginLocation, rewriters)
137+
for _, changed := range node.GetChanges() {
138+
for _, change := range changed.GetAllChanges() {
139+
if change == nil || change.Context == nil {
140+
continue
141+
}
142+
change.Context.DocumentLocation = rewriteDocumentLocation(change.Context.DocumentLocation, rewriters)
143+
}
144+
}
145+
for _, change := range node.RenderedChanges {
146+
if change == nil || change.Context == nil {
147+
continue
148+
}
149+
change.Context.DocumentLocation = rewriteDocumentLocation(change.Context.DocumentLocation, rewriters)
150+
}
151+
for _, change := range node.CleanedChanged {
152+
if change == nil || change.Context == nil {
153+
continue
154+
}
155+
change.Context.DocumentLocation = rewriteDocumentLocation(change.Context.DocumentLocation, rewriters)
156+
}
157+
158+
for _, child := range node.Children {
159+
walk(child)
160+
}
161+
}
162+
for _, node := range nodes {
163+
walk(node)
164+
}
165+
}
166+
167+
func rewriteDocumentLocation(raw string, rewriters []model.DocumentPathRewriter) string {
168+
if raw == "" {
169+
return raw
170+
}
171+
for _, rewrite := range rewriters {
172+
if rewrite == nil {
173+
continue
174+
}
175+
rewritten := rewrite(raw)
176+
if rewritten != raw {
177+
return rewritten
178+
}
179+
}
180+
return raw
181+
}
182+
94183
func emitCommitWarning(commitHash string, err error) {
95184
fmt.Fprintf(os.Stderr, "warning: commit %s: %s\n", commitHash, err)
96185
}

cmd/engine_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package cmd
5+
6+
import (
7+
"path/filepath"
8+
"strings"
9+
"testing"
10+
11+
v3 "github.com/pb33f/doctor/model/high/v3"
12+
"github.com/pb33f/openapi-changes/model"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestRunChangerator_RewritesChangedNodeOriginsToRepoRelativePaths(t *testing.T) {
18+
repoDir, fileName := createMovedRefGitSpecRepo(t)
19+
20+
commits, err := loadGitHistoryCommits(repoDir, fileName, summaryOpts{base: repoDir, noColor: true, limitTime: -1}, nil)
21+
require.NoError(t, err)
22+
require.NotEmpty(t, commits)
23+
24+
target := firstComparableCommit(commits)
25+
require.NotNil(t, target)
26+
27+
result, err := runChangerator(target, nil)
28+
require.NoError(t, err)
29+
require.NotNil(t, result)
30+
defer result.Release()
31+
32+
origins := collectChangedNodeOrigins(result.Changerator.ChangedNodes)
33+
require.NotEmpty(t, origins)
34+
35+
hasRepoRelativeOrigin := false
36+
for _, origin := range origins {
37+
assert.NotContains(t, origin, repoDir)
38+
assert.False(t, filepath.IsAbs(filepath.FromSlash(origin)), "origin should be repo-relative: %s", origin)
39+
if origin == "spec.yaml" || strings.HasPrefix(origin, "common") {
40+
hasRepoRelativeOrigin = true
41+
}
42+
}
43+
assert.True(t, hasRepoRelativeOrigin, "expected at least one repo-relative origin, got %v", origins)
44+
}
45+
46+
func firstComparableCommit(commits []*model.Commit) *model.Commit {
47+
for _, commit := range commits {
48+
if commit != nil && commit.Document != nil && commit.OldDocument != nil {
49+
return commit
50+
}
51+
}
52+
return nil
53+
}
54+
55+
func collectChangedNodeOrigins(nodes []*v3.Node) []string {
56+
seen := make(map[*v3.Node]struct{}, len(nodes))
57+
var origins []string
58+
59+
var walk func(*v3.Node)
60+
walk = func(node *v3.Node) {
61+
if node == nil {
62+
return
63+
}
64+
if _, ok := seen[node]; ok {
65+
return
66+
}
67+
seen[node] = struct{}{}
68+
69+
if node.Origin != nil {
70+
if node.Origin.AbsoluteLocation != "" {
71+
origins = append(origins, node.Origin.AbsoluteLocation)
72+
}
73+
if node.Origin.AbsoluteLocationValue != "" {
74+
origins = append(origins, node.Origin.AbsoluteLocationValue)
75+
}
76+
}
77+
if node.OriginLocation != "" {
78+
origins = append(origins, node.OriginLocation)
79+
}
80+
for _, child := range node.Children {
81+
walk(child)
82+
}
83+
}
84+
85+
for _, node := range nodes {
86+
walk(node)
87+
}
88+
return origins
89+
}

cmd/html_report_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,24 @@ func TestGenerateHTMLReport_LeftRightPreservesGitRefPaths(t *testing.T) {
7878
assert.Contains(t, content, `"modifiedPath":"HEAD:openapi.yaml"`)
7979
}
8080

81+
func TestGenerateHTMLReport_GitHistoryBasePathUsesRevisionScopedSiblingRefs(t *testing.T) {
82+
repoDir, fileName := createMovedRefGitSpecRepo(t)
83+
84+
commits, err := loadGitHistoryCommits(repoDir, fileName, summaryOpts{base: repoDir, noColor: true, limitTime: -1}, nil)
85+
require.NoError(t, err)
86+
require.NotEmpty(t, commits)
87+
88+
report, err := generateHTMLReport(commits, nil, true, repoDir, fileName)
89+
require.NoError(t, err)
90+
require.NotNil(t, report)
91+
92+
content := string(report)
93+
assert.Contains(t, content, "<!DOCTYPE html")
94+
assert.Contains(t, content, `"property":"$ref"`)
95+
assert.Contains(t, content, `"path":"$.paths['/thing'].get.responses['200'].content['application/json'].schema"`)
96+
assert.NotContains(t, content, `"origin":"`+repoDir)
97+
}
98+
8199
func TestGenerateHTMLReport_LeftRightSanitizesURLPaths(t *testing.T) {
82100
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
83101
if r.URL.Path == "/left.yaml" {
@@ -274,6 +292,35 @@ func TestBuildHTMLReportItems_SplitsStandardAndChangeExplorerGraphs(t *testing.T
274292
assert.Contains(t, explorerNodeWithChildChanges, "childChanges", "change explorer nodes render change summaries")
275293
}
276294

295+
func TestBuildHTMLReportItems_GitHistoryRewritesGraphOriginsToRepoRelativePaths(t *testing.T) {
296+
repoDir, fileName := createMovedRefGitSpecRepo(t)
297+
298+
commits, err := loadGitHistoryCommits(repoDir, fileName, summaryOpts{base: repoDir, noColor: true, limitTime: -1}, nil)
299+
require.NoError(t, err)
300+
require.NotEmpty(t, commits)
301+
302+
items, err := buildHTMLReportItems(commits, nil)
303+
require.NoError(t, err)
304+
require.Len(t, items, 1)
305+
306+
var graphNodes []map[string]any
307+
require.NoError(t, json.Unmarshal(items[0].Graph.Nodes, &graphNodes))
308+
309+
var origins []string
310+
for _, node := range graphNodes {
311+
origin, _ := node["origin"].(string)
312+
if origin != "" {
313+
origins = append(origins, origin)
314+
}
315+
}
316+
317+
require.NotEmpty(t, origins)
318+
for _, origin := range origins {
319+
assert.NotContains(t, origin, repoDir)
320+
assert.False(t, filepath.IsAbs(filepath.FromSlash(origin)), "origin should be repo-relative: %s", origin)
321+
}
322+
}
323+
277324
func TestBuildHTMLReportItems_ComposedSchemaTitleRemovalPreservesPropertyLabel(t *testing.T) {
278325
leftPath, rightPath := createComposedSchemaTitleRemovalSpecPair(t, "allOf")
279326

0 commit comments

Comments
 (0)