Support matching on arrays nested in the body #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshkeegan-form3
force-pushed
the
jk-arr-constraints
branch
8 times, most recently
from
324fa3e to
86a58fa
Compare
joshkeegan-form3
changed the title
joshkeegan-form3
force-pushed
the
jk-arr-constraints
branch
from
57f7fe9 to
b59a60b
Compare
ekosov-form3
previously approved these changes
Apr 4, 2024
internal/app/pactproxy/constraint.go
Outdated
| @@ -15,3 +18,34 @@ type interactionConstraint struct { | |||
| func (i interactionConstraint) Key() string { | |||
| return strings.Join([]string{i.Interaction, i.Path}, "_") | |||
| } | |||
|
|
|||
| func (i interactionConstraint) Check(expectedValues []interface{}, actualValue interface{}) error { | |||
There was a problem hiding this comment.
Does it need to be exported? 🤔
Suggested change
| func (i interactionConstraint) Check(expectedValues []interface{}, actualValue interface{}) error { | |
| func (i interactionConstraint) check(expectedValues []interface{}, actualValue interface{}) error { |
| expected := fmt.Sprintf(constraint.Format, values...) | ||
| if actual != expected { | ||
| violations = append(violations, fmt.Sprintf("value '%s' at path '%s' does not match constraint '%s'", actual, constraint.Path, expected)) | ||
| if err := constraint.Check(values, val); err != nil { |
There was a problem hiding this comment.
Nit: I slightly prefer original variable naming, tbh (i.e. actual vs. expected).
There was a problem hiding this comment.
expected & actual got moved inside Check. So values & val here are the raw data that was previously used to create those. They are very confusingly named though, and now expected & actual are freed up they work well here 👍
| case []interface{}: | ||
| // Create constraints for each element in the array. This allows matching rules to override them. | ||
| for j := range val { | ||
| i.addJSONConstraintsFromPact(fmt.Sprintf("%v[%v]", path, j), matchingRules, val[j]) |
There was a problem hiding this comment.
Just my curiosity - any reason not to use a bit more specific format (%s[%d]) as path and j are known to be string and int respectively?
There was a problem hiding this comment.
No reason, and it makes sense to use the more specific ones so that the arguments can't get accidentally swapped. Changed 👍
ekosov-form3
approved these changes
Apr 5, 2024
If there is an array nested within a JSON body on the pact, when a request was received and the interaction had its constraints evaluated, the constraint for the value of the array got skipped. This meant that if pact-proxy had interactions that differed only in the content of arrays, when requests were received it would match all of those interactions rather than the specific one that would have been matched had the contents of the array been considered. This was causing flaky tests in a project that used pact-proxy to wait for N of a specific interaction. The interaction was the same as another apart from the content of an array in the body. Since pact-proxy could not tell these apart, it would continue after N of either of these interactions, not N of the one the test needed to wait for. That randomly caused the next test to fail because interactions from the previous test were still ongoing. Now when constraints are added from the Pact, we generate constraints for each element in the array, as well as a length check. When evaluating constraints, we can then run these like we do for any other type. This allows for two interactions to be created that differ only in the contents of an array and pact-proxy is able to tell them apart. Generating constraints on a per-element basis is done to also allow for matching rules that apply to an individual array element. When matching rules are specified, pact-proxy does not enforce these so the constraint must not be generated for that element. As with non-array matching rules, if a request is received and the remaining constraints are met, the request is considered a match and proxied to Pact server, which will check matching rules.
Previously, a JSON request body being an array was a special-case. Now that arrays are handled nested within the JSON body, treat them as any other JSON. The constraints will now get correctly added as with any other value.
joshkeegan-form3
force-pushed
the
jk-arr-constraints
branch
from
95acf44 to
c36a434
Compare
|
Closing & re-opening as CI hasn't kicked off. Github had an outage earlier, so expect it was due to that. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If there is an array nested within a JSON body on the pact, when a request was received and the interaction had its constraints evaluated, the constraint for the value of the array got skipped. This meant that if pact-proxy had interactions that differed only in the content of arrays, when requests were received it would match all of those interactions rather than the specific one that would have been matched had the contents of the array been considered.
This was causing flaky tests in a project that used pact-proxy to wait for N of a specific interaction. The interaction was the same as another apart from the content of an array in the body. Since pact-proxy could not tell these apart, it would continue after N of either of these interactions, not N of the one the test needed to wait for. That randomly caused the next test to fail because interactions from the previous test were still ongoing.
Now when constraints are added from the Pact, we generate constraints for each element in the array, as well as a length check. When evaluating constraints, we can then run these like we do for any other type. This allows for two interactions to be created that differ only in the contents of an array and pact-proxy is able to tell them apart.
Generating constraints on a per-element basis is done to also allow for matching rules that apply to an individual array element. When matching rules are specified, pact-proxy does not enforce these so the constraint must not be generated for that element. As with non-array matching rules, if a request is received and the remaining constraints are met, the request is considered a match and proxied to Pact server, which will check matching rules.
Additionally (separate commit), constraints are now allowed when the JSON body is an array. Previously, a JSON request body being an array was a special-case. Now that arrays are handled nested within the JSON body, treat them as any other JSON. The constraints will now get correctly added as with any other value.