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

Alow to use "object" var in expressions when deserializing #827

Merged
merged 1 commit into from
Oct 19, 2017

Conversation

goetas
Copy link
Collaborator

@goetas goetas commented Oct 16, 2017

Q A
Bug fix? yes
New feature? no
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #826
License Apache-2.0

@goetas goetas merged commit ddb7d0d into master Oct 19, 2017
@goetas goetas deleted the allow-expression-exclusion-when-deserializing branch October 19, 2017 07:36
@mpoiriert
Copy link
Contributor

@goetas Why does the object is not available and must be set to null in a Deserialization context ?

Looking at the DeserializationGraphNavigator the object is available.

This cause some issue because if we want to do some security to allow edition of some attribute base on the currently connected user we can't.

Since the visitor is accessible from the context and that the current object could bet access from getCurrentObject we can have a work arround but the getCurrentObject is not part of the DeserializationVisitorInterface or VisitorInterface, both XMl and Json deserialization implements the method maybe it should be part of it ?

@goetas
Copy link
Collaborator Author

goetas commented May 8, 2020

Actually the object sometimes is available.

  • When doing the per-class exclusion, the object is not available as it has not yet been created (see
    if (null !== $this->exclusionStrategy && $this->exclusionStrategy->shouldSkipClass($metadata, $this->context)) {
    , the object gets created few lines later)
  • When doing the per-property exclusion the object is available, but is in a non consistent state, some props are deserialized and some not.

Given this contradictory situations, was much simpler to not make it available.
Can you please describe better your usecase?

@mpoiriert
Copy link
Contributor

I understand for the shouldSkip class, it make sens since it's about the class...

For the state of the object I thought about it and there is 2 way that I came up with:

  • Not great Clone the object and work on the clone itself for the test
  • Best way Loop trough all the property first just to check which property should be updated and then loop again to do the visit.

The use case itself is to have ACL on a object property level and not on a class property level.

I could submit a PR of the second solution if you want.

@mpoiriert
Copy link
Contributor

We will also need to come up with a none BC way to make sure that if some developer have done a check if object is null to apply some logic it will still work.

I thought of a few way but I don't want to make configuration too much complicated either...

@mpoiriert
Copy link
Contributor

Any interest in me making a PR of the second solution ? (I still need to implement it)

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.

2 participants