doctrine / data-fixtures

Doctrine2 ORM Data Fixtures Extensions
http://www.doctrine-project.org
MIT License
2.78k stars 224 forks source link

serialize/unserialize of ProxyReferenceRepository break custom Doctrine types #254

Closed JustBlackBird closed 4 years ago

JustBlackBird commented 7 years ago

I'm using Entities with columns of custom Doctrine types (ex. UUID type).

I have a user entity:

use Ramsey\Uuid\UuidInterface;

/**
 * @Entity
 */
class User
{
      /**
       * @Id
       * @Column(type="uuid")
       * @var UuidInterface
       */
      private $id;

      public function __construct(UuidInterface $id)
      {
          $this->id = $id;
      }

      public function getId()
      {
          return $this->id;
      }
}

When I run:

$id = Ramsey\Uuid\Uuid::uuid4();
$user = new User($id);

$repo = new ProxyReferenceRepository($em);
$repo->add('user', $user);

$serialized_data = $repo->serialize();
$repo_copy = new ProxyReferenceRepository($em);
$repo_copy->unserialize($serialized_data);

$user_copy = $repo_copy->getReference('user');

I get a broken User entity:

echo(is_string($user_copy->getId()) ? 'broken' : 'not sure'); // Outputs "broken"

In other words serialization inside of ProxyReferenceRepository does not unserialize correctly attached objects which are not Entities.


I've look at the code of ProxyReferenceRepository and saw this:

        $serializedData = json_encode(array(
            'references' => $simpleReferences,
            'identities' => $this->getIdentities(),
        ));

and this:

        $repositoryData = json_decode($serializedData, true);
        $references     = $repositoryData['references'];

The question is why json_encode/json_decode are used here instead of serialize/unserialize? As far as I can see such change does not break anything but make the library play nice with columns of custom Doctrine types.

JustBlackBird commented 7 years ago

Any comments on this?

Ocramius commented 7 years ago

@JustBlackBird it needs a test case, but yes, serialize and unserialize would be more appropriate.

JustBlackBird commented 7 years ago

@Ocramius Thanks for the response. I'll provide a PR soon.