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

[Workfow] Management of expressions in guard Listerner (and all others components) #24454

Closed
lyrixx opened this issue Oct 6, 2017 · 4 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Workflow

Comments

@lyrixx
Copy link
Member

lyrixx commented Oct 6, 2017

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

Hello.

In different places place of Symfony, we can configure things with an expression. Some exemples:


Yesterday I was working on #23499 and #23906 And I think we have an issue with the current implementation of the expression in the workflow component (and also with other component).

With theses 2 PR if someone wants to use only is_valid and if the security is not enable of if the user is not connected he will get an exception: https://github.com/symfony/symfony/pull/23906/files#diff-7e071774bef81a2a6e1b4e5d81d8ace8R59 ; That's not cool.

I guess we can fix this by moving this check right here

But still, I'm not really happy with that.


The EL has two modes: a runtime and a compiled mode. But now I fail to see how someone could use the compiled mode with the current implementation (in the workflow, but also with the @Security annotation).

So, we do have checks for the runtime mode but not for the compiled mode (just above). So here it can lead to a fatal error.

So if it's not possible at all to use the compiled in theses uses case, why don't we use propose injection in classes like https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Workflow/EventListener/ExpressionLanguage.php or https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/master/Security/ExpressionLanguage.php insteand of relying of the presence or not of a special var? Obviously That would made the compiled mode unable to work.

Note: I did not check for the integration between the EL and the Router or the DIC. But I guess thing are optimized there (or it could be)

Note2: In Blackfire there are place where you can configure some expression and when I implemented this feature I did not used the pattern used in Symfony. I disabled the compiled mode and I used propre inject in the ExpressionLanguage class (or FunctionProvider, it's the same things). It was really simpler and flexible.


I would like to use propre injection in the workflow component, and if it's applicable also in other component.

What do you think ?

@xabbuh xabbuh added RFC RFC = Request For Comments (proposals about features that you want to be discussed) Workflow labels Oct 6, 2017
@Simperfit
Copy link
Contributor

I agree, I think it could be great to have this in the workflow component. Do you still want to implement it ?

@lyrixx
Copy link
Member Author

lyrixx commented Dec 20, 2017

Sure.

@lyrixx
Copy link
Member Author

lyrixx commented Feb 7, 2018

If no one is against it, I will implement it this week.

@lyrixx
Copy link
Member Author

lyrixx commented Dec 4, 2018

Actually, this is complicated to implement it. Let's close this issue as it's not so important

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Workflow
Projects
None yet
Development

No branches or pull requests

3 participants