Skip to content

Commit 8263215

Browse files
Copilotbaywet
Copilot
and
baywet
authored
feat: Update x-copy behavior to require single source match per specification OAI/Overlay-Specificatino#PR #150 (#121)
* Initial plan * Initial commit: Update global.json SDK version Co-authored-by: baywet <[email protected]> * Update x-copy behavior to require single source match - Modified CopyNodes method to enforce exactly one copy source match - Updated error messages to reflect new validation requirements - Changed logic to copy same source to all targets (removed sequential pairing) - Updated existing tests that expected old sequential copy behavior - Added 4 new unit tests to validate single copy source requirement - All 53 tests now passing Co-authored-by: baywet <[email protected]> * Apply suggestions from code review * chore: formatting --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: baywet <[email protected]> Co-authored-by: Vincent Biret <[email protected]>
1 parent a9d5c31 commit 8263215

File tree

2 files changed

+170
-30
lines changed

2 files changed

+170
-30
lines changed

‎src/lib/Models/OverlayAction.cs‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -169,34 +169,27 @@ private bool CopyNodes(PathResult parseResult, JsonNode documentJsonNode, Overla
169169
overlayDiagnostic.Errors.Add(new OpenApiError(GetPointer(index), $"Copy target not found: '{Copy}'"));
170170
return false;
171171
}
172-
if (copyParseResult.Matches.Count < 1)
172+
if (copyParseResult.Matches.Count != 1)
173173
{
174-
overlayDiagnostic.Errors.Add(new OpenApiError(GetPointer(index), $"Copy target '{Copy}' must point to at least one JSON node"));
174+
overlayDiagnostic.Errors.Add(new OpenApiError(GetPointer(index), $"Copy JSON Path '{Copy}' must match exactly one result, but matched {copyParseResult.Matches.Count}"));
175175
return false;
176176
}
177177

178-
if (parseResult.Matches.Count != copyParseResult.Matches.Count)
179-
{
180-
overlayDiagnostic.Errors.Add(new OpenApiError(GetPointer(index), $"The number of matches for 'target' ({parseResult.Matches.Count}) and 'x-copy' ({copyParseResult.Matches.Count}) must be the same"));
181-
return false;
182-
}
183178
var matchValues = parseResult.Matches.Select(static m => m.Value).ToArray();
184179
if (matchValues.Any(static m => m is null))
185180
{
186181
overlayDiagnostic.Errors.Add(new OpenApiError(GetPointer(index), $"Target '{Target}' does not point to a valid JSON node"));
187182
return false;
188183
}
189-
var copyMatchValues = copyParseResult.Matches.Select(static m => m.Value).ToArray();
190-
if (copyMatchValues.Any(static m => m is null))
184+
if (copyParseResult.Matches[0].Value is not { } copyMatch)
191185
{
192186
overlayDiagnostic.Errors.Add(new OpenApiError(GetPointer(index), $"Copy target '{Copy}' does not point to a valid JSON node"));
193187
return false;
194188
}
195-
for (var i = 0; i < copyMatchValues.Length; i++)
189+
// Copy the same source to all targets
190+
foreach (var match in matchValues)
196191
{
197-
var match = matchValues[i];
198-
var copyMatch = copyMatchValues[i];
199-
MergeJsonNode(match!, copyMatch!, overlayDiagnostic);
192+
MergeJsonNode(match!, copyMatch, overlayDiagnostic);
200193
}
201194
return true;
202195
}

‎tests/lib/Serialization/OverlayActionTests.cs‎

Lines changed: 164 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -547,21 +547,22 @@ public void ApplyToDocument_ShouldCopyMultipleMatches()
547547
var overlayAction = new OverlayAction
548548
{
549549
Target = "$.paths[*].get.summary",
550-
Copy = "$.paths[*].get.operationId"
550+
Copy = "$.info.title"
551551
};
552552
var jsonNode = JsonNode.Parse("""
553553
{
554+
"info": {
555+
"title": "My API"
556+
},
554557
"paths": {
555558
"/users": {
556559
"get": {
557-
"summary": "Get users",
558-
"operationId": "listUsers"
560+
"summary": "Get users"
559561
}
560562
},
561563
"/posts": {
562564
"get": {
563-
"summary": "Get posts",
564-
"operationId": "listPosts"
565+
"summary": "Get posts"
565566
}
566567
}
567568
}
@@ -572,8 +573,8 @@ public void ApplyToDocument_ShouldCopyMultipleMatches()
572573
var result = overlayAction.ApplyToDocument(jsonNode, overlayDiagnostic, 0);
573574

574575
Assert.True(result);
575-
Assert.Equal("listUsers", jsonNode["paths"]?["/users"]?["get"]?["summary"]?.GetValue<string>());
576-
Assert.Equal("listPosts", jsonNode["paths"]?["/posts"]?["get"]?["summary"]?.GetValue<string>());
576+
Assert.Equal("My API", jsonNode["paths"]?["/users"]?["get"]?["summary"]?.GetValue<string>());
577+
Assert.Equal("My API", jsonNode["paths"]?["/posts"]?["get"]?["summary"]?.GetValue<string>());
577578
Assert.Empty(overlayDiagnostic.Errors);
578579
}
579580

@@ -624,7 +625,7 @@ public void ApplyToDocument_CopyShouldFailWhenCopyTargetNotFound()
624625
Assert.False(result);
625626
Assert.Single(overlayDiagnostic.Errors);
626627
Assert.Equal("$.actions[0]", overlayDiagnostic.Errors[0].Pointer);
627-
Assert.Equal("Copy target '$.nonexistent.field' must point to at least one JSON node", overlayDiagnostic.Errors[0].Message);
628+
Assert.Equal("Copy JSON Path '$.nonexistent.field' must match exactly one result, but matched 0", overlayDiagnostic.Errors[0].Message);
628629
}
629630

630631
[Fact]
@@ -656,31 +657,30 @@ public void ApplyToDocument_CopyShouldFailWhenCopyTargetHasNoMatches()
656657
Assert.False(result);
657658
Assert.Single(overlayDiagnostic.Errors);
658659
Assert.Equal("$.actions[0]", overlayDiagnostic.Errors[0].Pointer);
659-
Assert.Equal("Copy target '$.paths[*].get.nonexistent' must point to at least one JSON node", overlayDiagnostic.Errors[0].Message);
660+
Assert.Equal("Copy JSON Path '$.paths[*].get.nonexistent' must match exactly one result, but matched 0", overlayDiagnostic.Errors[0].Message);
660661
}
661662

662663
[Fact]
663-
public void ApplyToDocument_CopyShouldFailWhenMatchCountsDiffer()
664+
public void ApplyToDocument_CopyShouldFailWhenCopyHasMultipleMatches()
664665
{
665666
var overlayAction = new OverlayAction
666667
{
667668
Target = "$.paths[*].get.summary",
668-
Copy = "$.info.title"
669+
Copy = "$.paths[*].get.operationId"
669670
};
670671
var jsonNode = JsonNode.Parse("""
671672
{
672-
"info": {
673-
"title": "Test API"
674-
},
675673
"paths": {
676674
"/users": {
677675
"get": {
678-
"summary": "Get users"
676+
"summary": "Get users",
677+
"operationId": "listUsers"
679678
}
680679
},
681680
"/posts": {
682681
"get": {
683-
"summary": "Get posts"
682+
"summary": "Get posts",
683+
"operationId": "listPosts"
684684
}
685685
}
686686
}
@@ -693,7 +693,7 @@ public void ApplyToDocument_CopyShouldFailWhenMatchCountsDiffer()
693693
Assert.False(result);
694694
Assert.Single(overlayDiagnostic.Errors);
695695
Assert.Equal("$.actions[0]", overlayDiagnostic.Errors[0].Pointer);
696-
Assert.Equal("The number of matches for 'target' (2) and 'x-copy' (1) must be the same", overlayDiagnostic.Errors[0].Message);
696+
Assert.Equal("Copy JSON Path '$.paths[*].get.operationId' must match exactly one result, but matched 2", overlayDiagnostic.Errors[0].Message);
697697
}
698698

699699
[Fact]
@@ -909,4 +909,151 @@ public void ApplyToDocument_WorksWithSubSequentRemoval()
909909
Assert.Null(barFoo["$ref"]);
910910
Assert.Null(bazFoo["$ref"]);
911911
}
912+
913+
[Fact]
914+
public void ApplyToDocument_CopyShouldCopySingleSourceToMultipleTargets()
915+
{
916+
// This test validates the new behavior where a single copy source
917+
// is copied to multiple targets
918+
var overlayAction = new OverlayAction
919+
{
920+
Target = "$.paths[*].get.operationId",
921+
Copy = "$.info.title"
922+
};
923+
var jsonNode = JsonNode.Parse("""
924+
{
925+
"info": {
926+
"title": "Common Title"
927+
},
928+
"paths": {
929+
"/users": {
930+
"get": {
931+
"operationId": "oldOperationId1"
932+
}
933+
},
934+
"/posts": {
935+
"get": {
936+
"operationId": "oldOperationId2"
937+
}
938+
},
939+
"/comments": {
940+
"get": {
941+
"operationId": "oldOperationId3"
942+
}
943+
}
944+
}
945+
}
946+
""")!;
947+
var overlayDiagnostic = new OverlayDiagnostic();
948+
949+
var result = overlayAction.ApplyToDocument(jsonNode, overlayDiagnostic, 0);
950+
951+
Assert.True(result);
952+
Assert.Empty(overlayDiagnostic.Errors);
953+
// All three targets should now have the same value from the single copy source
954+
Assert.Equal("Common Title", jsonNode["paths"]?["/users"]?["get"]?["operationId"]?.GetValue<string>());
955+
Assert.Equal("Common Title", jsonNode["paths"]?["/posts"]?["get"]?["operationId"]?.GetValue<string>());
956+
Assert.Equal("Common Title", jsonNode["paths"]?["/comments"]?["get"]?["operationId"]?.GetValue<string>());
957+
}
958+
959+
[Fact]
960+
public void ApplyToDocument_CopyShouldFailWhenCopyMatchesZeroResults()
961+
{
962+
var overlayAction = new OverlayAction
963+
{
964+
Target = "$.info.title",
965+
Copy = "$.paths[*].get.nonexistent"
966+
};
967+
var jsonNode = JsonNode.Parse("""
968+
{
969+
"info": {
970+
"title": "Test API"
971+
},
972+
"paths": {
973+
"/users": {
974+
"get": {
975+
"summary": "Get users"
976+
}
977+
}
978+
}
979+
}
980+
""")!;
981+
var overlayDiagnostic = new OverlayDiagnostic();
982+
983+
var result = overlayAction.ApplyToDocument(jsonNode, overlayDiagnostic, 0);
984+
985+
Assert.False(result);
986+
Assert.Single(overlayDiagnostic.Errors);
987+
Assert.Equal("$.actions[0]", overlayDiagnostic.Errors[0].Pointer);
988+
Assert.Equal("Copy JSON Path '$.paths[*].get.nonexistent' must match exactly one result, but matched 0", overlayDiagnostic.Errors[0].Message);
989+
}
990+
991+
[Fact]
992+
public void ApplyToDocument_CopyShouldFailWhenCopyMatchesTwoResults()
993+
{
994+
var overlayAction = new OverlayAction
995+
{
996+
Target = "$.info.title",
997+
Copy = "$.paths[*].get.summary"
998+
};
999+
var jsonNode = JsonNode.Parse("""
1000+
{
1001+
"info": {
1002+
"title": "Test API"
1003+
},
1004+
"paths": {
1005+
"/users": {
1006+
"get": {
1007+
"summary": "Get users"
1008+
}
1009+
},
1010+
"/posts": {
1011+
"get": {
1012+
"summary": "Get posts"
1013+
}
1014+
}
1015+
}
1016+
}
1017+
""")!;
1018+
var overlayDiagnostic = new OverlayDiagnostic();
1019+
1020+
var result = overlayAction.ApplyToDocument(jsonNode, overlayDiagnostic, 0);
1021+
1022+
Assert.False(result);
1023+
Assert.Single(overlayDiagnostic.Errors);
1024+
Assert.Equal("$.actions[0]", overlayDiagnostic.Errors[0].Pointer);
1025+
Assert.Equal("Copy JSON Path '$.paths[*].get.summary' must match exactly one result, but matched 2", overlayDiagnostic.Errors[0].Message);
1026+
}
1027+
1028+
[Fact]
1029+
public void ApplyToDocument_CopyShouldSucceedWithExactlyOneMatch()
1030+
{
1031+
var overlayAction = new OverlayAction
1032+
{
1033+
Target = "$.paths['/users'].get.summary",
1034+
Copy = "$.info.description"
1035+
};
1036+
var jsonNode = JsonNode.Parse("""
1037+
{
1038+
"info": {
1039+
"title": "Test API",
1040+
"description": "API Description"
1041+
},
1042+
"paths": {
1043+
"/users": {
1044+
"get": {
1045+
"summary": "Old summary"
1046+
}
1047+
}
1048+
}
1049+
}
1050+
""")!;
1051+
var overlayDiagnostic = new OverlayDiagnostic();
1052+
1053+
var result = overlayAction.ApplyToDocument(jsonNode, overlayDiagnostic, 0);
1054+
1055+
Assert.True(result);
1056+
Assert.Empty(overlayDiagnostic.Errors);
1057+
Assert.Equal("API Description", jsonNode["paths"]?["/users"]?["get"]?["summary"]?.GetValue<string>());
1058+
}
9121059
}

0 commit comments

Comments
 (0)