Constructor relations writtable#2178
Conversation
6a300b3 to
5275c5a
Compare
implement constructor params relation denormalization drop identifier getter typehint because of doctrine listeners problem tests for cyclic relationship drop constructor cycle deps (impossible to handle), add iri and nullable param test test put method remove return types from test entities to pass low deps test fix cs fix phpstan adjust to changes in symfony/serializer PR
5275c5a to
e1733fd
Compare
37ad293 to
4309ffe
Compare
|
For now I've made But I think that it can be more guessable, consider example: class User
{
private $email;
private $password;
public function __construct(string $email) { $this->email = $email; }
public function getEmail(): string { return $this->email; }
public function getPassword(): string { return $this->password; }
public function setPassword(string $password): void { $this->password = $password; }
}Then documentation should looks like this: Such documentation behavior will allow us generating CRUD clients with distinction between what should appear in create form and what in update form, without the need to define serialization groups. If we're ok with this, I prefer to create it in different PR. For now non writable properties are included in documentation for |
dunglas
left a comment
There was a problem hiding this comment.
Only a minor change, then 👍 on my side
| return parent::denormalize($data, $class, $format, $context); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Please mark it as @internal because we'll remove it at some point.
| ]), | ||
| 'description' => new \ArrayObject([ | ||
| 'type' => 'string', | ||
| 'description' => 'This is a initializable but not writable property.', |
ef0a26d to
508e6ee
Compare
|
please, don't forget to update documentation. It's so painful when documentation is updated post-factum by someone who spends hours debugging, not in advance. thanks for this feature btw! |
| } else { | ||
| throw new MissingConstructorArgumentsException( | ||
| sprintf( | ||
| 'Cannot create an instance of %s from serialized data because its constructor requires parameter "%s" to be present.', |
There was a problem hiding this comment.
can this error be more consumer friendly on serialization? it's exposing implementation detail to the consumer which they don't care about: the class name, overly verbose message etc... all the consumer needs to know is that they didn't provide the some parameter.
There was a problem hiding this comment.
This code comes from symfony/serializer (see method comment). So I'm not sure if we should change it. But I've similar opinion. Ideally would be to show such errors the same way as validation errors (as map: property => error message). I think that non nullable and without default value constructor parameters are equivalent to class attributes with NotBlank constraint (with some exceptions) and errors should be presented the same way.
* test relations with writable constructor parameters implement constructor params relation denormalization drop identifier getter typehint because of doctrine listeners problem tests for cyclic relationship drop constructor cycle deps (impossible to handle), add iri and nullable param test test put method remove return types from test entities to pass low deps test fix cs fix phpstan adjust to changes in symfony/serializer PR * in swagger mark property as readOnly when is not writable and initializable * mark method internal * fix typo in test
* test relations with writable constructor parameters implement constructor params relation denormalization drop identifier getter typehint because of doctrine listeners problem tests for cyclic relationship drop constructor cycle deps (impossible to handle), add iri and nullable param test test put method remove return types from test entities to pass low deps test fix cs fix phpstan adjust to changes in symfony/serializer PR * in swagger mark property as readOnly when is not writable and initializable * mark method internal * fix typo in test
Connected with: #1749, symfony/symfony#28263
TODO: