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

[1.11][User] Reduce usage of deprecated \Serializable interface #13642

Closed
wants to merge 1 commit into from

Conversation

stloyd
Copy link
Contributor

@stloyd stloyd commented Feb 11, 2022

Reduce usage of deprecated \Serializable interface which is reported in PHP 8.1.

Deprecated: Sylius\Component\User\Model\User implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /srv/acme/vendor/sylius/sylius/src/Sylius/Component/User/Model/User.php on line 21

Added new __serialize/__unserialize methods to cover serialization & made serialize/unserialize internal to prevent usage of them (we can remove in next version).

Q A
Branch? 1.11
Bug fix? yes for PHP 8.1
New feature? no
BC breaks? no
Deprecations? fixed
License MIT

@stloyd stloyd requested a review from a team as a code owner February 11, 2022 16:28
@stloyd stloyd force-pushed the patch-10 branch 2 times, most recently from c1a238e to 067d5b7 Compare February 11, 2022 16:37
Replace usage of `serialize/unserialize` with `__serialize/__unserialize` methods
@stloyd stloyd changed the base branch from master to 1.11 February 11, 2022 16:57
@stloyd stloyd changed the title [User] Remove deprecated \Serializable interface [1.11][User] Remove deprecated \Serializable interface Feb 11, 2022
@stloyd stloyd changed the title [1.11][User] Remove deprecated \Serializable interface [1.11][User] Reduce usage of deprecated \Serializable interface Feb 11, 2022
@Nek-
Copy link
Contributor

Nek- commented Apr 1, 2022

@stloyd you have a static check failing :) . This PR is nice for PHP 8.1 full support. +1 for it to be merged! (I'm not going to duplicate this time 🙈 )

*/
public function unserialize($serialized): void
{
$data = unserialize($serialized);
$data = (array) unserialize((string) $serialized);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$data = (array) unserialize((string) $serialized);
$data = unserialize($data);
if (!is_array($data)) {
throw new UnexpectedValueException(sprintf('Cannot deserialize %s instance', self::class));
}

Should fix the Static checks.

Comment on lines +419 to +427
{
$this->password = $data['password'] ?? null;
$this->salt = $data['salt'] ?? null;
$this->usernameCanonical = $data['usernameCanonical'] ?? null;
$this->username = $data['username'] ?? null;
$this->locked = $data['locked'] ?? null;
$this->enabled = $data['enabled'] ?? null;
$this->id = $data['id'] ?? null;
$this->encoderName = $data['encoderName'] ?? null;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work, when we serialize we don't return a associative array !

lchrusciel added a commit that referenced this pull request May 6, 2022
…e` interface #13642  (sad270)

This PR was merged into the 1.11 branch.

Discussion
----------

Remove deprecated message of `\Serializable` interface in `User` entity which is reported in PHP 8.1.

```
Deprecated: Sylius\Component\User\Model\User implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /srv/acme/vendor/sylius/sylius/src/Sylius/Component/User/Model/User.php on line 21
```

- Added new `__serialize` and `__unserialize` methods to cover serialization
- Made `serialize` and `unserialize` internal to prevent usage of them (we can remove in next version).

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.11         |
| Bug fix?        | no                                                      |
| New feature?    | no                                                       |
| BC breaks?      | no                                                       |
| Deprecations?   | no/yes |
| Related tickets | #13642                      |
| License         | MIT                                                          |



Commits
-------

c94092c [User] Remove deprecated `\Serializable` interface
@stloyd stloyd closed this May 6, 2022
@stloyd stloyd deleted the patch-10 branch May 6, 2022 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants