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.

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

internal/fwschemadata: Rewrite SetValue semantic equality logic to ignore order #1064

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
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
53 changes: 34 additions & 19 deletions internal/fwschemadata/value_semantic_equality_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ func ValueSemanticEqualitySetElements(ctx context.Context, req ValueSemanticEqua
// Short circuit flag
updatedElements := false

// The underlying loop will mutate priorValueElements to avoid keeping
// duplicate semantically equal elements. Need the original length to avoid panicks
originalPriorElementsLength := len(priorValueElements)

// Loop through proposed elements by delegating to the recursive semantic
// equality logic. This ensures that recursion will catch a further
// underlying element type has its semantic equality logic checked, even if
Expand All @@ -136,33 +140,44 @@ func ValueSemanticEqualitySetElements(ctx context.Context, req ValueSemanticEqua
// Ensure new value always contains all of proposed new value
newValueElements[idx] = proposedNewValueElement

if idx >= len(priorValueElements) {
if idx >= originalPriorElementsLength {
continue
}

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

ValueSemanticEquality(ctx, elementReq, elementResp)
ValueSemanticEquality(ctx, elementReq, elementResp)

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

if resp.Diagnostics.HasError() {
return
}
if resp.Diagnostics.HasError() {
return
}

if elementResp.NewValue.Equal(elementReq.ProposedNewValue) {
continue
}
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

updatedElements = true
newValueElements[idx] = elementResp.NewValue
// 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
}
}

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