Skip to content

Commit

Permalink
internal/fwschemadata: Rewrite SetValue semantic equality logic to …
Browse files Browse the repository at this point in the history
…ignore order (#1064)

* quick rewrite of semantic equality for sets

* add unit tests for fix

* Revert "quick rewrite of semantic equality for sets"

This reverts commit 37749fd.

* fix the semantic equal custom type bump

* Revert "Revert "quick rewrite of semantic equality for sets""

This reverts commit b37dde1.

* changelog

* small change

* comment

* comment updates

* remove unnecessary check on prior element length
  • Loading branch information
austinvalle authored Feb 12, 2025
1 parent e1e6866 commit bf1d023
Show file tree
Hide file tree
Showing 4 changed files with 424 additions and 28 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/BUG FIXES-20250117-110109.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: BUG FIXES
body: 'internal/fwschemadata: Set semantic equality logic has been adjusted and will
now ignore order of elements during comparison.'
time: 2025-01-17T11:01:09.848503-05:00
custom:
Issue: "1061"
59 changes: 33 additions & 26 deletions internal/fwschemadata/value_semantic_equality_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,33 +136,40 @@ func ValueSemanticEqualitySetElements(ctx context.Context, req ValueSemanticEqua
// Ensure new value always contains all of proposed new value
newValueElements[idx] = proposedNewValueElement

if idx >= len(priorValueElements) {
continue
// Loop through all prior value elements and see if there are any semantically equal elements
for pIdx, priorValueElement := range priorValueElements {
elementReq := ValueSemanticEqualityRequest{
Path: req.Path.AtSetValue(proposedNewValueElement),
PriorValue: priorValueElement,
ProposedNewValue: proposedNewValueElement,
}
elementResp := &ValueSemanticEqualityResponse{
NewValue: elementReq.ProposedNewValue,
}

ValueSemanticEquality(ctx, elementReq, elementResp)

resp.Diagnostics.Append(elementResp.Diagnostics...)

if resp.Diagnostics.HasError() {
return
}

if elementResp.NewValue.Equal(elementReq.ProposedNewValue) {
// This prior value element didn't match, but there could be other elements that do
continue
}

// Prior state was kept, meaning that we found a semantically equal element
updatedElements = true

// Remove the semantically equal element from the slice of candidates
priorValueElements = append(priorValueElements[:pIdx], priorValueElements[pIdx+1:]...)

// Order doesn't matter, so we can just set the prior state element to this index
newValueElements[idx] = elementResp.NewValue
break
}

elementReq := ValueSemanticEqualityRequest{
Path: req.Path.AtSetValue(proposedNewValueElement),
PriorValue: priorValueElements[idx],
ProposedNewValue: proposedNewValueElement,
}
elementResp := &ValueSemanticEqualityResponse{
NewValue: elementReq.ProposedNewValue,
}

ValueSemanticEquality(ctx, elementReq, elementResp)

resp.Diagnostics.Append(elementResp.Diagnostics...)

if resp.Diagnostics.HasError() {
return
}

if elementResp.NewValue.Equal(elementReq.ProposedNewValue) {
continue
}

updatedElements = true
newValueElements[idx] = elementResp.NewValue
}

// No changes required if the elements were not updated.
Expand Down
Loading

0 comments on commit bf1d023

Please sign in to comment.