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.

Sign up for GitHub

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

Jump to bottom

Move type check when deserializing into the graph navigator #1080

Merged
merged 1 commit into from
May 3, 2019
Merged

Move type check when deserializing into the graph navigator #1080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/GraphNavigator/DeserializationGraphNavigator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ public function accept($data, ?array $type = null)
continue;
}

if (!$propertyMetadata->type) {
throw new RuntimeException(sprintf('You must define a type for %s::$%s.', $propertyMetadata->class, $propertyMetadata->name));
Copy link

@surikman surikman Jun 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@goetas Could you describe please, why was this check necessary?

My use-case:
I have an AbstractCommand => class in library outside of project and then I have own Command (e.g.: CreateUser) which extends AbstractCommand and my command has some properties (e.g.: username, email...)
I need to deserialize request such as {"username": "JohnDoe", "email":"john.doe@example.com"} into my CreateUser command (BUT AbstractCommand has special property metadata.

I don't need to use this property in my project, but Library works with this property.
When I updated from 3.0.1 to 3.1.0 I got an error message:
You must define a type for MyExternalLibraryAKACommandBus\AbstractCommand::$metadata

Is there any chance to skip this throwing exception instead of doing changes in this library?

Copy link

Choose a reason for hiding this comment

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

I opened an issue: #1102

}

$this->context->pushPropertyMetadata($propertyMetadata);
try {
$v = $this->visitor->visitProperty($propertyMetadata, $data);
Expand Down
16 changes: 2 additions & 14 deletions src/JsonDeserializationVisitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,35 +152,23 @@ public function startVisitingObject(ClassMetadata $metadata, object $object, arr
*/
public function visitProperty(PropertyMetadata $metadata, $data)
{
$name = $metadata->serializedName;

if (null === $data) {
return;
return null;
}

if (!\is_array($data)) {
throw new RuntimeException(sprintf('Invalid data %s (%s), expected "%s".', json_encode($data), $metadata->type['name'], $metadata->class));
}

if (true === $metadata->inline) {
if (!$metadata->type) {
throw new RuntimeException(sprintf(
'You must define a type for %s::$%s.',
$metadata->class,
$metadata->name
));
}
return $this->navigator->accept($data, $metadata->type);
}

$name = $metadata->serializedName;
if (!array_key_exists($name, $data)) {
throw new NotAcceptableException();
}

if (!$metadata->type) {
throw new RuntimeException(sprintf('You must define a type for %s::$%s.', $metadata->class, $metadata->name));
}

return null !== $data[$name] ? $this->navigator->accept($data[$name], $metadata->type) : null;
}

Expand Down
3 changes: 0 additions & 3 deletions src/XmlDeserializationVisitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,6 @@ public function visitProperty(PropertyMetadata $metadata, $data)
{
$name = $metadata->serializedName;

if (!$metadata->type) {
throw new RuntimeException(sprintf('You must define a type for %s::$%s.', $metadata->class, $metadata->name));
}
if (true === $metadata->inline) {
return $this->navigator->accept($data, $metadata->type);
}
Expand Down
6 changes: 6 additions & 0 deletions tests/Serializer/GraphNavigatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Doctrine\Common\Annotations\AnnotationReader;
use JMS\Serializer\Accessor\DefaultAccessorStrategy;
use JMS\Serializer\Annotation as Serializer;
use JMS\Serializer\Construction\UnserializeObjectConstructor;
use JMS\Serializer\DeserializationContext;
use JMS\Serializer\EventDispatcher\EventDispatcher;
Expand Down Expand Up @@ -163,6 +164,11 @@ protected function setUp()

class SerializableClass
{
/**
* @Serializer\Type("string")
*
* @var string
*/
public $foo = 'bar';
}

Expand Down