Skip to content

Commit ae4770b

Browse files
stofstof
authored
Validate value object required fields in the constructor (#1488)
* Refactor the code generating object properties * Refactor the generation of array getters Optional array fields still need to consider the property as nullable even though the getter will return an empty array as AWS does not treat absent lists the same than empty lists in input. * Add validation of required parameters in value object constructors * Remove checks for missing properties for value object request bodies This is already validated in the constructor now.
1 parent 44de083 commit ae4770b

File tree

6 files changed

+72
-31
lines changed

6 files changed

+72
-31
lines changed

‎src/Generator/InputGenerator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ private function inputClassRequestGetters(StructureShape $inputShape, ClassBuild
429429
if ($operation->hasBody()) {
430430
[$body['body'], $hasRequestBody, $overrideArgs] = $serializer->generateRequestBody($operation, $inputShape) + [null, null, []];
431431
if ($hasRequestBody) {
432-
[$returnType, $requestBody, $args] = $serializer->generateRequestBuilder($inputShape) + [null, null, []];
432+
[$returnType, $requestBody, $args] = $serializer->generateRequestBuilder($inputShape, true) + [null, null, []];
433433
if ('' === trim($requestBody)) {
434434
$body['body'] = '$body = "";';
435435
} else {

‎src/Generator/ObjectGenerator.php

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public function generate(StructureShape $shape, bool $forEndpoint = false): Clas
106106

107107
$serializer = $this->serializer->get($shape->getService());
108108
if ($this->isShapeUsedInput($shape)) {
109-
[$returnType, $requestBody, $args] = $serializer->generateRequestBuilder($shape) + [null, null, []];
109+
[$returnType, $requestBody, $args] = $serializer->generateRequestBuilder($shape, false) + [null, null, []];
110110
$method = $classBuilder->addMethod('requestBody')->setReturnType($returnType)->setBody($requestBody)->setPublic()->setComment('@internal');
111111
foreach ($args as $arg => $type) {
112112
$method->addParameter($arg)->setType($type);
@@ -191,36 +191,60 @@ private function namedConstructor(StructureShape $shape, ClassBuilder $classBuil
191191

192192
$constructor->addParameter('input')->setType('array');
193193

194+
// To throw an exception in an expression, we need to use a method to support PHP 7.x
195+
$needsThrowMethod = false;
196+
194197
$constructorBody = '';
195198
foreach ($shape->getMembers() as $member) {
196199
$memberShape = $member->getShape();
197200
if ($memberShape instanceof StructureShape) {
198201
$objectClass = $this->generate($memberShape);
199-
$constructorBody .= strtr('$this->PROPERTY = isset($input["NAME"]) ? CLASS::create($input["NAME"]) : null;' . "\n", ['PROPERTY' => GeneratorHelper::normalizeName($member->getName()), 'NAME' => $member->getName(), 'CLASS' => $objectClass->getName()]);
202+
$memberCode = strtr('CLASS::create($input["NAME"])', ['NAME' => $member->getName(), 'CLASS' => $objectClass->getName()]);
200203
} elseif ($memberShape instanceof ListShape) {
201204
$listMemberShape = $memberShape->getMember()->getShape();
202205

203206
// Check if this is a list of objects
204207
if ($listMemberShape instanceof StructureShape) {
205208
$objectClass = $this->generate($listMemberShape);
206-
$constructorBody .= strtr('$this->PROPERTY = isset($input["NAME"]) ? array_map([CLASS::class, "create"], $input["NAME"]) : null;' . "\n", ['PROPERTY' => GeneratorHelper::normalizeName($member->getName()), 'NAME' => $member->getName(), 'CLASS' => $objectClass->getName()]);
209+
$memberCode = strtr('array_map([CLASS::class, "create"], $input["NAME"])', ['NAME' => $member->getName(), 'CLASS' => $objectClass->getName()]);
207210
} else {
208-
$constructorBody .= strtr('$this->PROPERTY = $input["NAME"] ?? null;' . "\n", ['PROPERTY' => GeneratorHelper::normalizeName($member->getName()), 'NAME' => $member->getName()]);
211+
$memberCode = strtr('$input["NAME"]', ['NAME' => $member->getName()]);
209212
}
210213
} elseif ($memberShape instanceof MapShape) {
211214
$mapValueShape = $memberShape->getValue()->getShape();
212215

213216
if ($mapValueShape instanceof StructureShape) {
214217
$objectClass = $this->generate($mapValueShape);
215-
$constructorBody .= strtr('$this->PROPERTY = isset($input["NAME"]) ? array_map([CLASS::class, "create"], $input["NAME"]) : null;' . "\n", ['PROPERTY' => GeneratorHelper::normalizeName($member->getName()), 'NAME' => $member->getName(), 'CLASS' => $objectClass->getName()]);
218+
$memberCode = strtr('array_map([CLASS::class, "create"], $input["NAME"])', ['NAME' => $member->getName(), 'CLASS' => $objectClass->getName()]);
216219
} else {
217-
$constructorBody .= strtr('$this->PROPERTY = $input["NAME"] ?? null;' . "\n", ['PROPERTY' => GeneratorHelper::normalizeName($member->getName()), 'NAME' => $member->getName()]);
220+
$memberCode = strtr('$input["NAME"]', ['NAME' => $member->getName()]);
218221
}
219222
} else {
220-
$constructorBody .= strtr('$this->PROPERTY = $input["NAME"] ?? null;' . "\n", ['PROPERTY' => GeneratorHelper::normalizeName($member->getName()), 'NAME' => $member->getName()]);
223+
$memberCode = strtr('$input["NAME"]', ['NAME' => $member->getName()]);
224+
}
225+
if ($member->isRequired()) {
226+
$fallback = strtr('$this->throwException(new InvalidArgument(\'Missing required field "NAME".\'))', ['NAME' => $member->getName()]);
227+
$classBuilder->addUse(InvalidArgument::class);
228+
$needsThrowMethod = true;
229+
} else {
230+
$fallback = 'null';
221231
}
232+
$constructorBody .= strtr('$this->PROPERTY = isset($input["NAME"]) ? MEMBER_CODE : FALLBACK;' . "\n", [
233+
'PROPERTY' => GeneratorHelper::normalizeName($member->getName()),
234+
'NAME' => $member->getName(),
235+
'MEMBER_CODE' => $memberCode,
236+
'FALLBACK' => $fallback,
237+
]);
222238
}
223239
$constructor->setBody($constructorBody);
240+
241+
if ($needsThrowMethod) {
242+
$throwMethod = $classBuilder->addMethod('throwException');
243+
$throwMethod->setPrivate();
244+
$throwMethod->addComment('@return never');
245+
$throwMethod->addParameter('exception')->setType(\Throwable::class);
246+
$throwMethod->setBody('throw $exception;');
247+
}
224248
}
225249

226250
/**
@@ -249,12 +273,12 @@ private function addProperties(StructureShape $shape, ClassBuilder $classBuilder
249273
$enumClassName = $this->enumGenerator->generate($memberShape);
250274
$classBuilder->addUse($enumClassName->getFqdn());
251275
}
252-
$getterSetterNullable = true;
276+
$getterSetterNullable = null;
253277

254278
if ($memberShape instanceof StructureShape) {
255279
$this->generate($memberShape);
256280
} elseif ($memberShape instanceof MapShape) {
257-
$nullable = $getterSetterNullable = false;
281+
$getterSetterNullable = false;
258282
$mapKeyShape = $memberShape->getKey()->getShape();
259283
if ('string' !== $mapKeyShape->getType()) {
260284
throw new \RuntimeException('Complex maps are not supported');
@@ -271,7 +295,7 @@ private function addProperties(StructureShape $shape, ClassBuilder $classBuilder
271295
$classBuilder->addUse($enumClassName->getFqdn());
272296
}
273297
} elseif ($memberShape instanceof ListShape) {
274-
$nullable = $getterSetterNullable = false;
298+
$getterSetterNullable = false;
275299
$memberShape->getMember()->getShape();
276300

277301
if (($memberShape = $memberShape->getMember()->getShape()) instanceof StructureShape) {
@@ -297,7 +321,10 @@ private function addProperties(StructureShape $shape, ClassBuilder $classBuilder
297321
$deprecation = strtr('@trigger_error(\sprintf(\'The property "NAME" of "%s" is deprecated by AWS.\', __CLASS__), E_USER_DEPRECATED);', ['NAME' => $member->getName()]);
298322
}
299323

300-
if ($getterSetterNullable) {
324+
$nullable = $nullable ?? !$member->isRequired();
325+
$getterSetterNullable = $getterSetterNullable ?? $nullable;
326+
327+
if ($getterSetterNullable || !$nullable) {
301328
$method->setBody($deprecation . strtr('
302329
return $this->PROPERTY;
303330
', [
@@ -311,11 +338,10 @@ private function addProperties(StructureShape $shape, ClassBuilder $classBuilder
311338
]));
312339
}
313340

314-
$nullable = $nullable ?? !$member->isRequired();
315341
if ($parameterType && $parameterType !== $returnType && (empty($memberClassNames) || $memberClassNames[0]->getName() !== $parameterType)) {
316-
$method->addComment('@return ' . $parameterType . ($nullable ? '|null' : ''));
342+
$method->addComment('@return ' . $parameterType . ($getterSetterNullable ? '|null' : ''));
317343
}
318-
$method->setReturnNullable($nullable);
344+
$method->setReturnNullable($getterSetterNullable);
319345
}
320346

321347
foreach ($forEndpointProps as $key => $ok) {

‎src/Generator/RequestSerializer/QuerySerializer.php

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ public function generateRequestBody(Operation $operation, StructureShape $shape)
6969
]), true];
7070
}
7171

72-
public function generateRequestBuilder(StructureShape $shape): array
72+
public function generateRequestBuilder(StructureShape $shape, bool $needsChecks): array
7373
{
74-
$body = implode("\n", array_map(function (StructureMember $member) {
74+
$body = implode("\n", array_map(function (StructureMember $member) use ($needsChecks) {
7575
if (null !== $member->getLocation()) {
7676
return '';
7777
}
@@ -84,10 +84,15 @@ public function generateRequestBuilder(StructureShape $shape): array
8484
MEMBER_CODE';
8585
$inputElement = '$v';
8686
} elseif ($member->isRequired()) {
87-
$body = 'if (null === $v = $this->PROPERTY) {
88-
throw new InvalidArgument(sprintf(\'Missing parameter "NAME" for "%s". The value cannot be null.\', __CLASS__));
87+
if ($needsChecks) {
88+
$body = 'if (null === $v = $this->PROPERTY) {
89+
throw new InvalidArgument(sprintf(\'Missing parameter "NAME" for "%s". The value cannot be null.\', __CLASS__));
90+
}
91+
MEMBER_CODE';
92+
} else {
93+
$body = '$v = $this->PROPERTY;
94+
MEMBER_CODE';
8995
}
90-
MEMBER_CODE';
9196
$inputElement = '$v';
9297
} else {
9398
$body = 'if (null !== $v = $this->PROPERTY) {

‎src/Generator/RequestSerializer/RestJsonSerializer.php

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ public function generateRequestBody(Operation $operation, StructureShape $shape)
6868
return ['$bodyPayload = $this->requestBody(); $body = empty($bodyPayload) ? "{}" : \json_encode($bodyPayload, ' . \JSON_THROW_ON_ERROR . ');', true];
6969
}
7070

71-
public function generateRequestBuilder(StructureShape $shape): array
71+
public function generateRequestBuilder(StructureShape $shape, bool $needsChecks): array
7272
{
73-
$body = implode("\n", array_map(function (StructureMember $member) {
73+
$body = implode("\n", array_map(function (StructureMember $member) use ($needsChecks) {
7474
if (null !== $member->getLocation()) {
7575
return '';
7676
}
@@ -83,10 +83,15 @@ public function generateRequestBuilder(StructureShape $shape): array
8383
MEMBER_CODE';
8484
$inputElement = '$v';
8585
} elseif ($member->isRequired()) {
86-
$body = 'if (null === $v = $this->PROPERTY) {
87-
throw new InvalidArgument(sprintf(\'Missing parameter "NAME" for "%s". The value cannot be null.\', __CLASS__));
86+
if ($needsChecks) {
87+
$body = 'if (null === $v = $this->PROPERTY) {
88+
throw new InvalidArgument(sprintf(\'Missing parameter "NAME" for "%s". The value cannot be null.\', __CLASS__));
89+
}
90+
MEMBER_CODE';
91+
} else {
92+
$body = '$v = $this->PROPERTY;
93+
MEMBER_CODE';
8894
}
89-
MEMBER_CODE';
9095
$inputElement = '$v';
9196
} else {
9297
$body = 'if (null !== $v = $this->PROPERTY) {

‎src/Generator/RequestSerializer/RestXmlSerializer.php

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ public function generateRequestBody(Operation $operation, StructureShape $shape)
9393
', true, ['node' => \DOMNode::class]];
9494
}
9595

96-
public function generateRequestBuilder(StructureShape $shape): array
96+
public function generateRequestBuilder(StructureShape $shape, bool $needsChecks): array
9797
{
98-
$body = implode("\n", array_map(function (StructureMember $member) {
98+
$body = implode("\n", array_map(function (StructureMember $member) use ($needsChecks) {
9999
if (null !== $member->getLocation()) {
100100
return '';
101101
}
@@ -108,10 +108,15 @@ public function generateRequestBuilder(StructureShape $shape): array
108108
MEMBER_CODE';
109109
$inputElement = '$v';
110110
} elseif ($member->isRequired()) {
111-
$body = 'if (null === $v = $this->PROPERTY) {
112-
throw new InvalidArgument(sprintf(\'Missing parameter "NAME" for "%s". The value cannot be null.\', __CLASS__));
111+
if ($needsChecks) {
112+
$body = 'if (null === $v = $this->PROPERTY) {
113+
throw new InvalidArgument(sprintf(\'Missing parameter "NAME" for "%s". The value cannot be null.\', __CLASS__));
114+
}
115+
MEMBER_CODE';
116+
} else {
117+
$body = '$v = $this->PROPERTY;
118+
MEMBER_CODE';
113119
}
114-
MEMBER_CODE';
115120
$inputElement = '$v';
116121
} else {
117122
$body = 'if (null !== $v = $this->PROPERTY) {

‎src/Generator/RequestSerializer/Serializer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function generateRequestBody(Operation $operation, StructureShape $shape)
2929
*
3030
* @return array{0: string, 1: string, 2?: array<string, string>}
3131
*/
32-
public function generateRequestBuilder(StructureShape $shape): array;
32+
public function generateRequestBuilder(StructureShape $shape, bool $needsChecks): array;
3333

3434
public function getHeaders(Operation $operation): string;
3535
}

0 commit comments

Comments
 (0)