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

Instructions for upgrading from addData in 1.x don't work #1030

Closed
GKFX opened this issue Jan 12, 2019 · 7 comments
Closed

Instructions for upgrading from addData in 1.x don't work #1030

GKFX opened this issue Jan 12, 2019 · 7 comments

Comments

@GKFX
Copy link

GKFX commented Jan 12, 2019

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no

The provided instructions for upgrading from addData in 1.x don't work. This is code from a project using the Symfony bundle. Instructions were (UPGRADING.md)

Removed deprecated method JsonSerializationVisitor::addData, use :visitProperty(new StaticPropertyMetadata('', 'name', 'value'), null) instead

Steps required to reproduce the problem

  1. My old code (extract from https://github.com/camdram/camdram/blob/38216b3fe4c3b87d0f2f74d73a2521a315c0d920/src/Acts/CamdramApiBundle/Serializer/JsonEventSubscriber.php)
public function onPostSerialize(ObjectEvent $event)
{
    // ...
    $visitor->addData('_links', $linkJson); // $linkJson is an array
    $visitor->addData('_type', strtolower($class->getShortName()));
}
  1. My new code, now with library version v2.0.2, following instructions provided in UPDATING.md:
$visitor->visitProperty(new StaticPropertyMetadata('', '_links', $linkJson), null);
  1. My new code, having ignored the documentation and read the source instead ;)
$visitor->visitProperty(new StaticPropertyMetadata('', '_links', null), $linkJson);

Expected Result

    "_links": {
        "self": "\/shows\/2020-the-phantom-of-the-opera",
        "venue": "\/venues\/demo-venue",
        "society": "\/societies\/society-10"
    },
    "_type": "show"

formed part of the output with with 1.x and was expected with method 2.

Actual Result

The expected JSON did not appear with method 2. It did appear with method 3.

Is this a bug in the code or a bug in the documentation? I.e. should I continue using method 3 or wait until a fix is released that makes method 2 work?

GKFX added a commit to camdram/camdram that referenced this issue Jan 12, 2019
Bit of a weird one, I think there's a bug in the upgrade instructions and have asked the maintainer to clarify (schmittjoh/serializer#1030).

Bumps [jms/serializer-bundle](https://github.com/schmittjoh/JMSSerializerBundle) from 2.4.3 to 3.0.1.
- [Release notes](https://github.com/schmittjoh/JMSSerializerBundle/releases)
- [Changelog](https://github.com/schmittjoh/JMSSerializerBundle/blob/master/CHANGELOG.md)
- [Commits](schmittjoh/JMSSerializerBundle@2.4.3...3.0.1)
@GKFX GKFX changed the title instructions for upgrading from addData in 1.x don't work Instructions for upgrading from addData in 1.x don't work Jan 12, 2019
@goetas
Copy link
Collaborator

goetas commented Jan 12, 2019

hmm, that's interesting...

$visitor->visitProperty(new StaticPropertyMetadata('', '_links', $linkJson), null);

should be the right one... at least looking at

if ($metadata instanceof StaticPropertyMetadata) {

@Sander-Toonen
Copy link

I have the same experience.
Are you sure the documentation is right?

@CHTJonas
Copy link

Bump!

@goetas
Copy link
Collaborator

goetas commented Jun 25, 2019

Option 3 was the right way and #1066 updated the documentation

@CHTJonas
Copy link

That's grand - thanks!

@faysh
Copy link

faysh commented Jun 27, 2019

What's the forth parameter ($groups) on StaticPropertyMetadata for? I added groups, but it doesn't exclude property at all. What I'm doing wrong?

@afkernohan
Copy link

It fails silently if your StaticPropertyMetadata is set up incorrectly:

    try {
        $v = $this->navigator->accept($v, $metadata->type);
    } catch (NotAcceptableException $e) {
        return;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants