Skip to content

Fix missing deref. #515

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

Closed
wants to merge 2 commits into from
Closed

Fix missing deref. #515

wants to merge 2 commits into from

Conversation

Danack
Copy link

@Danack Danack commented May 21, 2018

Fixes #514

I realise this could do with some tests.

Please can you direct me to a similar existent test to copy and modify, or at least say where an appropriate test might go.

@Danack
Copy link
Author

Danack commented May 21, 2018

broken test appears not related to change.

Copy link
Contributor

@erayd erayd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit tests which verify the behaviour of this new code. These tests should fail when your change is not present, and pass when it is.

@erayd
Copy link
Contributor

erayd commented May 21, 2018

Please can you direct me to a similar existent test to copy and modify, or at least say where an appropriate test might go.

Tests for this should probably go in ArraysTest.php. You'll see many example tests in the same folder.

@tarunlalwani
Copy link

tarunlalwani commented May 22, 2018

@Danack , @erayd , seems like the PR is not needed as such. The issue was in the usage of library.

The validation was run like

$jsonValidator->check($endpointData, (object)$schemaForPath);

Which worked for a single object but not for a array type schema. And the usage should have been like

$jsonValidator->check($endpointData, $schemaForPath);

See updated answer on

https://stackoverflow.com/a/50449472/2830850

Probably a thought, that you should have some check that people don't mix stdClass and array or something that could be just documented to avoid such issues in future

@erayd
Copy link
Contributor

erayd commented May 22, 2018

@tarunlalwani Well spotted - thanks. I've had a play with the code in #514, and it seems that aliasSchema() (function in the test code used to reformat the swagger input) is causing the problem by returning assoc arrays instead of objects. If it's adjusted to return an object, then the validator behaves as expected.

@Danack
Copy link
Author

Danack commented Jun 6, 2018

I'll close this PR as it was based on me being dumb.

@Danack Danack closed this Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants