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

Feature request - extend 'match each' to work with 'contains deep' #2170

Closed
anupamck opened this issue Nov 4, 2022 · 4 comments
Closed

Feature request - extend 'match each' to work with 'contains deep' #2170

anupamck opened this issue Nov 4, 2022 · 4 comments
Assignees

Comments

@anupamck
Copy link
Contributor

anupamck commented Nov 4, 2022

Currently, match each doesn't work with contains deep.

Scenario:
    * def original = [ { a: 1, arr: [ { b: 2, c: 3 }, { b: 3, c: 4 } ] } ]
    * def expected = { a: 1, arr: [ { b: '#number', c: '#number' } ] }
    # This fails
    * match each original contains deep expected

This is in line with the documentation, given that there is a section for match contains deep, but no corresponding section for match each contains deep. However, I think this feature would be of great use, especially in performing schema validation for complex JSON arrays with the powerful flexibility of contains deep.

@ptrthomas
Copy link
Member

tagging as help wanted - should be easy if you refer fix for #2093

@vincent-hennig
Copy link
Contributor

I did a bit of digging together with @anupamck and it seems to us that most of the functionality for each contains deep is already implemented. We found that MatchStep.getType seems to simply be missing a case for this.

private static Match.Type getType(boolean each, boolean not, boolean contains, boolean only, boolean any, boolean deep) {
if (each) {
if (contains) {
if (only) {
return Match.Type.EACH_CONTAINS_ONLY;
}
if (any) {
return Match.Type.EACH_CONTAINS_ANY;
}
return not ? Match.Type.EACH_NOT_CONTAINS : Match.Type.EACH_CONTAINS;
}
return not ? Match.Type.EACH_NOT_EQUALS : Match.Type.EACH_EQUALS;
}
if (contains) {
if (only) {
return deep ? Match.Type.CONTAINS_ONLY_DEEP : Match.Type.CONTAINS_ONLY;
}
if (any) {
return Match.Type.CONTAINS_ANY;
}
if (deep) {
if (not) {
throw new RuntimeException("'!contains deep' is not yet supported, use 'contains deep' instead");
}
return Match.Type.CONTAINS_DEEP;
}
return not ? Match.Type.NOT_CONTAINS : Match.Type.CONTAINS;
}
return not ? Match.Type.NOT_EQUALS : Match.Type.EQUALS;
}

Here is the Change that I would propose:

vincent-hennig@24564d2#diff-b849d563bd2a8fccf84bcb162901dc2ba61ab6aeaebc743e0648361f2db019a4

@ptrthomas
Copy link
Member

@vincent-hennig sounds right to me ! how about you attempt a PR ? if all existing tests pass, we are all good. and you can add a couple of tests for this case

@ptrthomas
Copy link
Member

1.3.1 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants