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

ExclusionStrategy backward compatibility break #843

Closed
hmichal-dev opened this issue Nov 23, 2017 · 8 comments
Closed

ExclusionStrategy backward compatibility break #843

hmichal-dev opened this issue Nov 23, 2017 · 8 comments

Comments

@hmichal-dev
Copy link

hmichal-dev commented Nov 23, 2017

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

Steps required to reproduce the problem

In v.1.9.0 ExclusionStrategy::shouldSkipProperty skips only main properties , in 1.9.1 it skips properties of embed object too. It's a bug or backward compatibility break (shouldn't be due to Semantic Versioning)

Example config

// Product Entity
	/**
	 * @var int
	 * @JMS\Groups({"prod"})
	 */
	protected $id;
	
	/**
	 * @var Category
	 * @JMS\Groups({"prod"})
	 */
	protected $cat;
	
	/**
	 * @var string
	 * @JMS\Groups({"prod"})
	 */
	protected $shortName;

	
// Category Entity

	/**
	 * @var int
	 * @JMS\Groups({"prod"})
	 */
	protected $id;
	
	/**
	 * @var string
	 * @JMS\Groups({"prod"})
	 */
	protected $shortName;

Expected Result (version v1.9.0)

  • If I exclude shortName, Shop::shortName is not excluded

Actual Result (version v1.9.1 and 1.9.2)

  • If I exclude shortName, Shop::shortName is excluded
@goetas
Copy link
Collaborator

goetas commented Nov 23, 2017

are you serializing to json or xml?

@hmichal-dev
Copy link
Author

To JSON

@hmichal-dev
Copy link
Author

It's nice feature (if it's not a bug), but due to backward compatibility break it can cause problems

I solve problem by adding this code to shouldSkipProperty

    if ($navigatorContext->getDepth() > 1) {
        return false;
    }

@goetas
Copy link
Collaborator

goetas commented Nov 23, 2017

I'm not sure is a bug/bc break what you are reporting. as you can see 1.9.1...1.9.2 have almost no changes (especially if you are dealing with json

@hmichal-dev
Copy link
Author

Sorry, I just checked, the last version I had used have been 1.9.0, not 1.9.1.

@goetas
Copy link
Collaborator

goetas commented Nov 23, 2017

1.9.0...1.9.2 is not much different anyway

@hmichal-dev
Copy link
Author

Ok. It's related to some other library. It is a coincidence, but last package update caused problem i thought it is related to Serializer. I'm sorry for the trouble

@goetas
Copy link
Collaborator

goetas commented Nov 23, 2017

👍

@goetas goetas closed this as completed Nov 23, 2017
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

No branches or pull requests

2 participants