-
Notifications
You must be signed in to change notification settings - Fork 5
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
Added new JsonPath condition #17
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manofearth Many thanks for this great improvement. Please check the broken integration tests: https://scrutinizer-ci.com/g/mcustiel/phiremock-server/inspections/d23a5f44-8028-4b4b-bee5-e7787f679fae
When it's fixed I will approve because this looks awesome.
I love this new feature. Again, thank you very much.
$value = $requestData; | ||
|
||
foreach ($path as $key) { | ||
if (!is_array($value) || !isset($value[$key])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[question] From the code I assume we can also use notation like: users.0.name
in case of indexed arrays. Is that correct? If yes, it would be nice to document it and have a test for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are absolutely right about array access notation - users.0.name
will work for accessing array elements. I will add documentation and test to demonstrate this functionality.
@mcustiel Thank you very much for such a positive feedback! I'm really glad you find this feature valuable. About the failing tests - I believe the issue is related to the dependency on phiremock-common package. I have created a separate PR there with the necessary changes to support jsonPath matching: mcustiel/phiremock-common#17. The tests are failing in scrutinizer most likely because it's still using the old version of phiremock-common without jsonPath support. Here's what I propose:
Does this plan work for you? |
Sounds good. I also think that the broken tests were related to the unmerged changes on phiremock-common. I approved your PR there and merged it, so the build can maybe use the latest release. |
@mcustiel Thank you again for the positive feedback and quick response! I've added documentation and tests for array access notation. However, the tests are still failing but for a different reason now - it's related to PHP 7.2 compatibility. I accidentally left a trailing comma in a parameter list in phiremock-common which is not supported in PHP 7.2. I've created a fix in PR mcustiel/phiremock-common#18 to address this issue. Could you please review and merge it? After that, if you could restart the scrutinizer build for this repository (phiremock-server), the tests should pass successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great Job. Thank you very much
@mcustiel Thanks for the quick turnaround in approving this PR! One last puzzle piece remains - would you please review this PR to phiremock-client which adds the corresponding jsonPath support to complete the feature? |
Add JSON Path request condition filter
This PR adds a new feature that allows filtering requests by checking values in specific JSON paths of the request body. This is particularly useful when you need to match nested JSON structures in requests.
Changes include:
The feature supports all standard matchers (isEqualTo, matches, contains, isSameString) and uses simple dot notation for specifying JSON paths. Use cases include:
All tests are passing