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

Find suitable value for oneOf property in parameters. Fix #166 #168

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sloory
Copy link

@sloory sloory commented May 13, 2022

No description provided.

@sloory sloory changed the title Find suitable value for oneOf property in parameters. Fix #166 Find suitable value for oneOf property in parameters. May 13, 2022
@sloory sloory changed the title Find suitable value for oneOf property in parameters. Find suitable value for oneOf property in parameters. Fix #166 May 13, 2022
@sloory
Copy link
Author

sloory commented May 13, 2022

this PR fix #166

Copy link
Member

@scaytrase scaytrase left a comment

Choose a reason for hiding this comment

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

Needs some fixing for negative cases

@@ -181,6 +183,14 @@ protected function convertToSerializationStyle($value, ?CebeSchema $schema)
if ($schema && $this->style === self::STYLE_DEEP_OBJECT) {
foreach ($value as $key => &$val) {
$childSchema = $this->getChildSchema($schema, (string) $key);

if (isset($childSchema->oneOf)) {
Copy link
Member

Choose a reason for hiding this comment

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

Looks like in this case we should immediately stop validation if we have filled oneOf and none of the schemas matched in findSuitableOneOf.


try {
$validator->validate($psrRequest);
$this->assertTrue(true);
Copy link
Member

Choose a reason for hiding this comment

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

this is not proper way how to deal with exceptions and zero assertions. proper way would be (without any try..catch)

$validator->validate($psrRequest);
$this->addToAssrtionsCount(1);

any exception thrown here should break the test and if validate throws one assertTrue(true) is not called anyway

*/
final class Issue166Test extends BaseValidatorTest
{
public function testIssue166(): void
Copy link
Member

Choose a reason for hiding this comment

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

some negative cases required

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.

In case of specified oneOf property in parameters library does not find suitable
2 participants