Skip to content

Commit

Permalink
[PHP] Improve: Make validation strict (#7724)
Browse files Browse the repository at this point in the history 
* Add test case which reproduce the problem

refs swagger-api/swagger-codegen#7686 (comment)
> 1. We should pass true as 3rd argument of in_array()

* Add test case for setter

* Strict validation

* Update samples

* Tweak expected value according to changes in #7723
  • Loading branch information
@ackintosh @wing328
ackintosh authored and wing328 committed Mar 25, 2018
1 parent 6d88d07 commit d58835e
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 26 deletions.
6 changes: 3 additions & 3 deletions modules/swagger-codegen/src/main/resources/php/model_generic.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^pa
{{#isEnum}}
{{^isContainer}}
$allowedValues = $this->{{getter}}AllowableValues();
if (!is_null($this->container['{{name}}']) && !in_array($this->container['{{name}}'], $allowedValues)) {
if (!is_null($this->container['{{name}}']) && !in_array($this->container['{{name}}'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for '{{name}}', must be one of '%s'",
implode("', '", $allowedValues)
Expand Down Expand Up @@ -274,7 +274,7 @@ class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^pa
{{#isEnum}}
{{^isContainer}}
$allowedValues = $this->{{getter}}AllowableValues();
if (!is_null($this->container['{{name}}']) && !in_array($this->container['{{name}}'], $allowedValues)) {
if (!is_null($this->container['{{name}}']) && !in_array($this->container['{{name}}'], $allowedValues, true)) {
return false;
}
{{/isContainer}}
Expand Down Expand Up @@ -344,7 +344,7 @@ class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^pa
{{#isEnum}}
$allowedValues = $this->{{getter}}AllowableValues();
{{^isContainer}}
if ({{^required}}!is_null(${{name}}) && {{/required}}!in_array(${{{name}}}, $allowedValues)) {
if ({{^required}}!is_null(${{name}}) && {{/required}}!in_array(${{{name}}}, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for '{{name}}', must be one of '%s'",
Expand Down
6 changes: 3 additions & 3 deletions samples/client/petstore/php/SwaggerClient-php/lib/Model/EnumArrays.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public function listInvalidProperties()
$invalidProperties = [];

$allowedValues = $this->getJustSymbolAllowableValues();
if (!is_null($this->container['just_symbol']) && !in_array($this->container['just_symbol'], $allowedValues)) {
if (!is_null($this->container['just_symbol']) && !in_array($this->container['just_symbol'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for 'just_symbol', must be one of '%s'",
implode("', '", $allowedValues)
Expand All @@ -246,7 +246,7 @@ public function valid()
{

$allowedValues = $this->getJustSymbolAllowableValues();
if (!is_null($this->container['just_symbol']) && !in_array($this->container['just_symbol'], $allowedValues)) {
if (!is_null($this->container['just_symbol']) && !in_array($this->container['just_symbol'], $allowedValues, true)) {
return false;
}
return true;
Expand All @@ -273,7 +273,7 @@ public function getJustSymbol()
public function setJustSymbol($just_symbol)
{
$allowedValues = $this->getJustSymbolAllowableValues();
if (!is_null($just_symbol) && !in_array($just_symbol, $allowedValues)) {
if (!is_null($just_symbol) && !in_array($just_symbol, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'just_symbol', must be one of '%s'",
Expand Down
24 changes: 12 additions & 12 deletions samples/client/petstore/php/SwaggerClient-php/lib/Model/EnumTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ public function listInvalidProperties()
$invalidProperties = [];

$allowedValues = $this->getEnumStringAllowableValues();
if (!is_null($this->container['enum_string']) && !in_array($this->container['enum_string'], $allowedValues)) {
if (!is_null($this->container['enum_string']) && !in_array($this->container['enum_string'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for 'enum_string', must be one of '%s'",
implode("', '", $allowedValues)
Expand All @@ -289,23 +289,23 @@ public function listInvalidProperties()
$invalidProperties[] = "'enum_string_required' can't be null";
}
$allowedValues = $this->getEnumStringRequiredAllowableValues();
if (!is_null($this->container['enum_string_required']) && !in_array($this->container['enum_string_required'], $allowedValues)) {
if (!is_null($this->container['enum_string_required']) && !in_array($this->container['enum_string_required'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for 'enum_string_required', must be one of '%s'",
implode("', '", $allowedValues)
);
}

$allowedValues = $this->getEnumIntegerAllowableValues();
if (!is_null($this->container['enum_integer']) && !in_array($this->container['enum_integer'], $allowedValues)) {
if (!is_null($this->container['enum_integer']) && !in_array($this->container['enum_integer'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for 'enum_integer', must be one of '%s'",
implode("', '", $allowedValues)
);
}

$allowedValues = $this->getEnumNumberAllowableValues();
if (!is_null($this->container['enum_number']) && !in_array($this->container['enum_number'], $allowedValues)) {
if (!is_null($this->container['enum_number']) && !in_array($this->container['enum_number'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for 'enum_number', must be one of '%s'",
implode("', '", $allowedValues)
Expand All @@ -325,22 +325,22 @@ public function valid()
{

$allowedValues = $this->getEnumStringAllowableValues();
if (!is_null($this->container['enum_string']) && !in_array($this->container['enum_string'], $allowedValues)) {
if (!is_null($this->container['enum_string']) && !in_array($this->container['enum_string'], $allowedValues, true)) {
return false;
}
if ($this->container['enum_string_required'] === null) {
return false;
}
$allowedValues = $this->getEnumStringRequiredAllowableValues();
if (!is_null($this->container['enum_string_required']) && !in_array($this->container['enum_string_required'], $allowedValues)) {
if (!is_null($this->container['enum_string_required']) && !in_array($this->container['enum_string_required'], $allowedValues, true)) {
return false;
}
$allowedValues = $this->getEnumIntegerAllowableValues();
if (!is_null($this->container['enum_integer']) && !in_array($this->container['enum_integer'], $allowedValues)) {
if (!is_null($this->container['enum_integer']) && !in_array($this->container['enum_integer'], $allowedValues, true)) {
return false;
}
$allowedValues = $this->getEnumNumberAllowableValues();
if (!is_null($this->container['enum_number']) && !in_array($this->container['enum_number'], $allowedValues)) {
if (!is_null($this->container['enum_number']) && !in_array($this->container['enum_number'], $allowedValues, true)) {
return false;
}
return true;
Expand All @@ -367,7 +367,7 @@ public function getEnumString()
public function setEnumString($enum_string)
{
$allowedValues = $this->getEnumStringAllowableValues();
if (!is_null($enum_string) && !in_array($enum_string, $allowedValues)) {
if (!is_null($enum_string) && !in_array($enum_string, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'enum_string', must be one of '%s'",
Expand Down Expand Up @@ -400,7 +400,7 @@ public function getEnumStringRequired()
public function setEnumStringRequired($enum_string_required)
{
$allowedValues = $this->getEnumStringRequiredAllowableValues();
if (!in_array($enum_string_required, $allowedValues)) {
if (!in_array($enum_string_required, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'enum_string_required', must be one of '%s'",
Expand Down Expand Up @@ -433,7 +433,7 @@ public function getEnumInteger()
public function setEnumInteger($enum_integer)
{
$allowedValues = $this->getEnumIntegerAllowableValues();
if (!is_null($enum_integer) && !in_array($enum_integer, $allowedValues)) {
if (!is_null($enum_integer) && !in_array($enum_integer, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'enum_integer', must be one of '%s'",
Expand Down Expand Up @@ -466,7 +466,7 @@ public function getEnumNumber()
public function setEnumNumber($enum_number)
{
$allowedValues = $this->getEnumNumberAllowableValues();
if (!is_null($enum_number) && !in_array($enum_number, $allowedValues)) {
if (!is_null($enum_number) && !in_array($enum_number, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'enum_number', must be one of '%s'",
Expand Down
6 changes: 3 additions & 3 deletions samples/client/petstore/php/SwaggerClient-php/lib/Model/Order.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public function listInvalidProperties()
$invalidProperties = [];

$allowedValues = $this->getStatusAllowableValues();
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues)) {
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for 'status', must be one of '%s'",
implode("', '", $allowedValues)
Expand All @@ -257,7 +257,7 @@ public function valid()
{

$allowedValues = $this->getStatusAllowableValues();
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues)) {
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues, true)) {
return false;
}
return true;
Expand Down Expand Up @@ -380,7 +380,7 @@ public function getStatus()
public function setStatus($status)
{
$allowedValues = $this->getStatusAllowableValues();
if (!is_null($status) && !in_array($status, $allowedValues)) {
if (!is_null($status) && !in_array($status, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'status', must be one of '%s'",
Expand Down
6 changes: 3 additions & 3 deletions samples/client/petstore/php/SwaggerClient-php/lib/Model/Pet.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public function listInvalidProperties()
$invalidProperties[] = "'photo_urls' can't be null";
}
$allowedValues = $this->getStatusAllowableValues();
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues)) {
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues, true)) {
$invalidProperties[] = sprintf(
"invalid value for 'status', must be one of '%s'",
implode("', '", $allowedValues)
Expand All @@ -269,7 +269,7 @@ public function valid()
return false;
}
$allowedValues = $this->getStatusAllowableValues();
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues)) {
if (!is_null($this->container['status']) && !in_array($this->container['status'], $allowedValues, true)) {
return false;
}
return true;
Expand Down Expand Up @@ -416,7 +416,7 @@ public function getStatus()
public function setStatus($status)
{
$allowedValues = $this->getStatusAllowableValues();
if (!is_null($status) && !in_array($status, $allowedValues)) {
if (!is_null($status) && !in_array($status, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'status', must be one of '%s'",
Expand Down
4 changes: 2 additions & 2 deletions samples/client/petstore/php/SwaggerClient-php/phpunit.xml.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

<filter>
<whitelist processUncoveredFilesFromWhitelist="true">
<directory suffix=".php">./lib\Api</directory>
<directory suffix=".php">./lib\Model</directory>
<directory suffix=".php">./lib/Api</directory>
<directory suffix=".php">./lib/Model</directory>
</whitelist>
</filter>
</phpunit>
7 changes: 7 additions & 0 deletions samples/client/petstore/php/SwaggerClient-php/test/Model/EnumTestTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ public function testPropertyEnumString()
{
}

/**
* Test attribute "enum_string_required"
*/
public function testPropertyEnumStringRequired()
{
}

/**
* Test attribute "enum_integer"
*/
Expand Down
24 changes: 24 additions & 0 deletions samples/client/petstore/php/SwaggerClient-php/tests/EnumTestTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@ public function testPossibleValues()
$this->assertSame(EnumTest::ENUM_NUMBER_MINUS_1_DOT_2, -1.2);
}

public function testStrictValidation()
{
$enum = new EnumTest([
'enum_string' => 0,
]);

$this->assertFalse($enum->valid());

$expected = [
"invalid value for 'enum_string', must be one of 'UPPER', 'lower', ''",
"'enum_string_required' can't be null",
];
$this->assertSame($expected, $enum->listInvalidProperties());
}

/**
* @expectedException \InvalidArgumentException
*/
public function testThrowExceptionWhenInvalidAmbiguousValueHasPassed()
{
$enum = new EnumTest();
$enum->setEnumString(0);
}

public function testNonRequiredPropertyIsOptional()
{
$enum = new EnumTest([
Expand Down

0 comments on commit d58835e

Please sign in to comment.