Skip to content

[Serializer] Add versionning #50850

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
Open
wants to merge 2 commits into
base: 7.3
Choose a base branch
from
Open

[Serializer] Add versionning #50850

wants to merge 2 commits into from

Conversation

micoli
Copy link

@micoli micoli commented Jul 2, 2023 •
edited by OskarStark
Loading

Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #30848
License MIT
Doc PR symfony/symfony-docs#...
  • Add version annotation/attribute to be set on the property holding the version of the normalized representation (only one property can be set as Version)
  • Add VersionConstraint annotation/attribute. That attribute control restriction on normalisation/denormalization depending on the value of the property that hold Version attribute. Inner properties are since and until. At least one of them has to be set.
  • Either Version.version, VersionConstraint.since and VersionConstraint.until are expressed as semver string, and must be compliant with version_compare function. A property restricted by a constraint is defined as : notDefined < since|null <= defined <= until|null < notDefined

Behavior and naming are inspired by https://jmsyst.com/libs/serializer/master/reference/annotations#since and https://jmsyst.com/libs/serializer/master/reference/annotations#until

Logic is as follow:

  • Normalization
    A restricted property by a VersionConstraint is not added to the normalized representation if the constraint is not satisfied
  • Denormalization
    A restricted property by a VersionConstraint is set to null if the constraint is not satisfied or to the value if the constraint is satisfied

Example of usage

class VersionAwareObject
{
    private $foo;

    /**
     * @Version
     */
    public string $version;

    /**
     * @VersionConstraint(
     *     since="1.1",
     *     until="1.5"
     * )
     */
    public ?string $name;

    /**
     * @VersionConstraint(
     *     since="1.6"
     * )
     */
    public ?string $lastName;
}

I don't see any Breaking Changes up to now

@micoli micoli requested a review from dunglas as a code owner July 2, 2023 16:45
@carsonbot carsonbot added this to the 6.4 milestone Jul 2, 2023
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@micoli micoli force-pushed the add-versionning-to-serializer branch 2 times, most recently from b49f144 to 2781c01 Compare July 2, 2023 17:03
@antonkomarev
Copy link

How to deal with cases when in the future version the field should become not nullable or it's type should be changed?

@asispts
Copy link
Contributor

asispts commented Jul 2, 2023

Similar PR #38478

@micoli
Copy link
Author

micoli commented Jul 2, 2023

Similar PR #38478

I'v only seen #35617, not this one. Hence this PR and mine have the same purpose, I can close mine if it's desired

@micoli
Copy link
Author

micoli commented Jul 2, 2023

How to deal with cases when in the future version the field should become not nullable or it's type should be changed?

Indeed, to have versioned field, due to the fact it can once be null, then by design those versionConstrained fields should be defined as nullable. I can add a check on that when reading field metadatas

@micoli micoli force-pushed the add-versionning-to-serializer branch from 2781c01 to 0b35462 Compare July 2, 2023 18:47
@micoli
Copy link
Author

micoli commented Jul 2, 2023

How to deal with cases when in the future version the field should become not nullable or it's type should be changed?

I've added a check to enforce the fact the field should be, by design, either without typehint either with a nullable type.

Regarding the type change of the field, i've no clue how to enforce it. Do you have an idea?

@OskarStark OskarStark changed the title Add versionning to serializer [Serializer] Add versionning Jul 3, 2023
@94noni
Copy link
Contributor

94noni commented Jul 3, 2023

hello (just passing by)
is it relevant to add annotation as the PR target v6 with php v8.1 as minimal requirement?
perhaps only native attribute is enough

@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Feature Serializer Status: Needs Review
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Serializer] Serialization versioning
9 participants
@micoli @carsonbot @antonkomarev @asispts @94noni @fabpot @nicolas-grekas @OskarStark @xabbuh