[WIP] [DependencyInjection] added support for expressions in the service container #8850

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
@fabpot
Owner

fabpot commented Aug 25, 2013

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7352
License MIT
Doc PR not yet

This PR introduces a way to inject expression in the definition of a service.

Here is a small usage example:

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Expression;

class Foo
{
    private $value;

    public function __construct($value)
    {
        $this->value = $value;
    }

    public function getBarValue()
    {
        return $this->value;
    }
}

class Bar
{
    public function getValue()
    {
        return 'bar';
    }
}

$c = new ContainerBuilder();
$c->register('bar', 'Bar');
$c->register('foo', 'Foo')->addArgument(new Expression('bar', 'value'));

echo $c->get('foo')->getBarValue()."\n";

Here is the XML version:

    <services>
        <service id="foo" class="Foo">
            <argument type="expression" id="bar" path="value" />
        </service>
        <service id="bar" class="Bar" />
    </services>

And the YAML version:

services:
    bar:
        class: Bar

    foo:
        class: Foo
        arguments: [@bar::value]

The Expression constructor takes a service id and a property path (as defined by the Symfony PropertyAccess component). Using the Symfony PropertyAccess component gives you a lot of flexibility in the way you get data from your objects.

Expressions are not limited to service arguments, you can use them everywhere (like in a method call, a configurator, ...).

Todo

  • update documentation
  • update all loaders and dumpers
  • add tests

Open questions:

  • Do we allow some cusomtization of the property access object (like support for __call?)
@lolautruche

This comment has been minimized.

Show comment Hide comment
@lolautruche

lolautruche Aug 25, 2013

Contributor

+1 👍
I like this implementation 😃

Contributor

lolautruche commented Aug 25, 2013

+1 👍
I like this implementation 😃

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Aug 25, 2013

Contributor

yeah. i think this goes in the right direction.

Contributor

lsmith77 commented Aug 25, 2013

yeah. i think this goes in the right direction.

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

I've implemented the YAML and XML loaders (see the description for examples). For the YAML loader, we have to introduce another convention (@bar::value), not sure which one would be better.

Owner

fabpot commented Aug 25, 2013

I've implemented the YAML and XML loaders (see the description for examples). For the YAML loader, we have to introduce another convention (@bar::value), not sure which one would be better.

@asm89

This comment has been minimized.

Show comment Hide comment
@asm89

asm89 Aug 25, 2013

Contributor

How would this work with a dumped container? (php one)

Other than that, interesting to see work on dynamic parameters in the DIC. I'd really like to put database information in for example $_ENV instead of getting it compiled into cache files.

Contributor

asm89 commented Aug 25, 2013

How would this work with a dumped container? (php one)

Other than that, interesting to see work on dynamic parameters in the DIC. I'd really like to put database information in for example $_ENV instead of getting it compiled into cache files.

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

@asm89 Quite easy: I've just pushed the PHP dumper to let you see how it would work (some details need to be changed but that works).

Owner

fabpot commented Aug 25, 2013

@asm89 Quite easy: I've just pushed the PHP dumper to let you see how it would work (some details need to be changed but that works).

@fmeynard

This comment has been minimized.

Show comment Hide comment
@fmeynard

fmeynard Aug 25, 2013

+1 :)

+1 :)

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

One limitation (from PropertyAccess) is that you can't pass arguments to method calls. So you can do $foo->bar(), but you cannot do $foo->bar('baz'). If support would be added to the PropertyAccess component, it would be automatically available in the service container.

Note that supporting something like $foo->bar('value of dic parameter baz') would be much more difficult to implement and would probably need that the service container reimplements its own expression language.

Owner

fabpot commented Aug 25, 2013

One limitation (from PropertyAccess) is that you can't pass arguments to method calls. So you can do $foo->bar(), but you cannot do $foo->bar('baz'). If support would be added to the PropertyAccess component, it would be automatically available in the service container.

Note that supporting something like $foo->bar('value of dic parameter baz') would be much more difficult to implement and would probably need that the service container reimplements its own expression language.

@asm89

This comment has been minimized.

Show comment Hide comment
@asm89

asm89 Aug 25, 2013

Contributor

@fabpot Nice, looks easy indeed.
@bschussek Any numbers on the performance of PropertyAccessor? In other places I'm not too worried about using the component, but the dumped container should be super optimized imo.

Contributor

asm89 commented Aug 25, 2013

@fabpot Nice, looks easy indeed.
@bschussek Any numbers on the performance of PropertyAccessor? In other places I'm not too worried about using the component, but the dumped container should be super optimized imo.

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

@asm89 As this is not a feature that people will use that often, I'm not too concerned about performance.

Owner

fabpot commented Aug 25, 2013

@asm89 As this is not a feature that people will use that often, I'm not too concerned about performance.

@webmozart

This comment has been minimized.

Show comment Hide comment
@webmozart

webmozart Aug 25, 2013

Contributor

Looks good! :)

@asm89 The performance is not optimal yet, because PropertyAccessor is using reflection, which is slow. Static code generation for property paths is planned, but I don't know when I (or anyone else) will be able to do this.

@fabpot For reference, in #7726 I suggested a syntax for using property paths in validator.yml:

properties:
    value:
        - Range:
            min: @path.to.min

This would access the "min" property path at the currently validated object. I'm not sure whether the two proposals are consistent.

I wonder what happens when you add a property path as part of a method call?

    my_service:
        calls:
            - [mySetter, [@service::path]]

I suppose this works? What happens if I want to access a path on the same object? (I know this might be a super rare edge case but still, for consistency, we should support it)

I think you should add support for the following syntax:

    my_service:
        calls:
            - [mySetter, [::some.nested.path]]

This would access the property path on the service itself. The syntax for validator.yml could then be configured consistently:

properties:
    value:
        - Range:
            min: ::path.to.min
Contributor

webmozart commented Aug 25, 2013

Looks good! :)

@asm89 The performance is not optimal yet, because PropertyAccessor is using reflection, which is slow. Static code generation for property paths is planned, but I don't know when I (or anyone else) will be able to do this.

@fabpot For reference, in #7726 I suggested a syntax for using property paths in validator.yml:

properties:
    value:
        - Range:
            min: @path.to.min

This would access the "min" property path at the currently validated object. I'm not sure whether the two proposals are consistent.

I wonder what happens when you add a property path as part of a method call?

    my_service:
        calls:
            - [mySetter, [@service::path]]

I suppose this works? What happens if I want to access a path on the same object? (I know this might be a super rare edge case but still, for consistency, we should support it)

I think you should add support for the following syntax:

    my_service:
        calls:
            - [mySetter, [::some.nested.path]]

This would access the property path on the service itself. The syntax for validator.yml could then be configured consistently:

properties:
    value:
        - Range:
            min: ::path.to.min
@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

Some answers to @bschussek questions:

  • @ in the service container is already used in the YAML format to reference a service;
  • You can use an expression anywhere in a service definition (so that works in a method call);
  • Calling a path on the same object is supported, just use its service id:
my_service:
        calls:
            - [mySetter, [@my_service::some.nested.path]]

Or we can add a special self word to reference the current service (but I don't like this idea).

Owner

fabpot commented Aug 25, 2013

Some answers to @bschussek questions:

  • @ in the service container is already used in the YAML format to reference a service;
  • You can use an expression anywhere in a service definition (so that works in a method call);
  • Calling a path on the same object is supported, just use its service id:
my_service:
        calls:
            - [mySetter, [@my_service::some.nested.path]]

Or we can add a special self word to reference the current service (but I don't like this idea).

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

I was not clear enough in my first answer. @path.to.min would not work in the service container as it would mean the path.to.min service.

Owner

fabpot commented Aug 25, 2013

I was not clear enough in my first answer. @path.to.min would not work in the service container as it would mean the path.to.min service.

@webmozart

This comment has been minimized.

Show comment Hide comment
@webmozart

webmozart Aug 25, 2013

Contributor

That's why I suggested ::path.to.min :)

Contributor

webmozart commented Aug 25, 2013

That's why I suggested ::path.to.min :)

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

... which I don't like ;)

Owner

fabpot commented Aug 25, 2013

... which I don't like ;)

@webmozart

This comment has been minimized.

Show comment Hide comment
@webmozart

webmozart Aug 25, 2013

Contributor

But it would allow for a consistent (=intuitive) syntax in validator.yml... I don't think our users enjoy if the same functionality has a slightly different syntax in different places of the framework.

Contributor

webmozart commented Aug 25, 2013

But it would allow for a consistent (=intuitive) syntax in validator.yml... I don't think our users enjoy if the same functionality has a slightly different syntax in different places of the framework.

@cryptocompress

This comment has been minimized.

Show comment Hide comment
@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Aug 25, 2013

Contributor

does spring have anything similar?

Contributor

lsmith77 commented Aug 25, 2013

does spring have anything similar?

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

@lsmith77 Spring has this: http://static.springsource.org/spring/docs/3.2.x/spring-framework-reference/html/expressions.html which allow for more complex expression. I named the new feature Expression in reference to Spring but I'm open to any suggestion.

Owner

fabpot commented Aug 25, 2013

@lsmith77 Spring has this: http://static.springsource.org/spring/docs/3.2.x/spring-framework-reference/html/expressions.html which allow for more complex expression. I named the new feature Expression in reference to Spring but I'm open to any suggestion.

@cryptocompress

This comment has been minimized.

Show comment Hide comment
@cryptocompress

cryptocompress Aug 25, 2013

as of $id; in there, name should be something very specific. e.g.: PropertyAccessExpression

http://static.springsource.org/spring/docs/3.2.x/javadoc-api/org/springframework/expression/Expression.html

as of $id; in there, name should be something very specific. e.g.: PropertyAccessExpression

http://static.springsource.org/spring/docs/3.2.x/javadoc-api/org/springframework/expression/Expression.html

@Crell

This comment has been minimized.

Show comment Hide comment
@Crell

Crell Aug 25, 2013

Contributor

I'm a bit unclear of the use case here. Do you have a non-foo/bar example of where it would be used, and what it does?

I think from the examples in this thread the idea is to allow a dependency of a service to be fulfilled not by another service, but by the result of a method call on another service. Am I correct in that, or way off base? (If way off base, correct me and ignore the rest of this comment.)

If so, I don't see how I'd specify arguments to said method. If that were resolved, though, I can think of a number of places where Drupal could make use of this, and in fact I really would like to. Because I could see lots of uses for it, the performance of the dumped container really would be relevant as I could see us using that a lot.

Contributor

Crell commented Aug 25, 2013

I'm a bit unclear of the use case here. Do you have a non-foo/bar example of where it would be used, and what it does?

I think from the examples in this thread the idea is to allow a dependency of a service to be fulfilled not by another service, but by the result of a method call on another service. Am I correct in that, or way off base? (If way off base, correct me and ignore the rest of this comment.)

If so, I don't see how I'd specify arguments to said method. If that were resolved, though, I can think of a number of places where Drupal could make use of this, and in fact I really would like to. Because I could see lots of uses for it, the performance of the dumped container really would be relevant as I could see us using that a lot.

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Aug 25, 2013

Owner

@Crell you are correct, the idea is to be able to inject the result of a method call on another service. As I said in a previous comment, the path is limited to what is currently supported by the Symfony PropertyAccess component, which does not support arguments to method calls.

Can you give us some examples of usage you envision for Drupal? That would make things more clearer than just foo/bar/baz?

Owner

fabpot commented Aug 25, 2013

@Crell you are correct, the idea is to be able to inject the result of a method call on another service. As I said in a previous comment, the path is limited to what is currently supported by the Symfony PropertyAccess component, which does not support arguments to method calls.

Can you give us some examples of usage you envision for Drupal? That would make things more clearer than just foo/bar/baz?

@aysheka

This comment has been minimized.

Show comment Hide comment
@aysheka

aysheka Aug 25, 2013

Yeah, good idea. +1

aysheka commented Aug 25, 2013

Yeah, good idea. +1

@Crell

This comment has been minimized.

Show comment Hide comment
@Crell

Crell Aug 26, 2013

Contributor

Well, the Drupal use case that comes to mind is our configuration system. (We're not using the Symfony Config component, just the YAML parser.)

We have a config.factory service, which acts as a repository for all configuration. Configuration is stored in configuration objects, which map 1:1 to YAML files. We have a lot of services right now that do something along the lines of the following:

doer.stuff:
  class: Stuff
  arguments: ['@url_generator', '@config_factory']
classs Stuff {
  protected $generator;
  protected $config;

  public function __construct(UrlGenerator $generator, ConfigFactory $config_factory) {
    $this->generator = $generator;
    $this->config = $config_factory->get('stuff.things');
  }
}

Some objects are storing the whole config factory instead, too, and then calling $this->configFactory->get('stuff.things') in some other method.

That makes writing unit tests more complicated since you have 2 levels of object to mock. What I think would be better is something along the lines of (YAML syntax totally made up here):

doer.stuff:
  class: Stuff
  arguments: ['@url_generator', '@config_factory::get(stuff.things)']
classs Stuff {
  protected $generator;
  protected $config;

  public function __construct(UrlGenerator $generator, Config $config) {
    $this->generator = $generator;
    $this->config = $config;
  }
}

That makes the Stuff class easier to unit test, as we need only mock the Config object rather than mocking a ConfigFactory object to return a mocked Config object.

(Hopefully that makes sense.)

Contributor

Crell commented Aug 26, 2013

Well, the Drupal use case that comes to mind is our configuration system. (We're not using the Symfony Config component, just the YAML parser.)

We have a config.factory service, which acts as a repository for all configuration. Configuration is stored in configuration objects, which map 1:1 to YAML files. We have a lot of services right now that do something along the lines of the following:

doer.stuff:
  class: Stuff
  arguments: ['@url_generator', '@config_factory']
classs Stuff {
  protected $generator;
  protected $config;

  public function __construct(UrlGenerator $generator, ConfigFactory $config_factory) {
    $this->generator = $generator;
    $this->config = $config_factory->get('stuff.things');
  }
}

Some objects are storing the whole config factory instead, too, and then calling $this->configFactory->get('stuff.things') in some other method.

That makes writing unit tests more complicated since you have 2 levels of object to mock. What I think would be better is something along the lines of (YAML syntax totally made up here):

doer.stuff:
  class: Stuff
  arguments: ['@url_generator', '@config_factory::get(stuff.things)']
classs Stuff {
  protected $generator;
  protected $config;

  public function __construct(UrlGenerator $generator, Config $config) {
    $this->generator = $generator;
    $this->config = $config;
  }
}

That makes the Stuff class easier to unit test, as we need only mock the Config object rather than mocking a ConfigFactory object to return a mocked Config object.

(Hopefully that makes sense.)

@denderello

This comment has been minimized.

Show comment Hide comment
@denderello

denderello Aug 26, 2013

Contributor

👍

Contributor

denderello commented Aug 26, 2013

👍

@docteurklein

This comment has been minimized.

Show comment Hide comment
@docteurklein

docteurklein Aug 26, 2013

Contributor

Isn't this functionality a shortcut to something already doable via service factory ?

services:
    bar:
        class: Bar

    bar_value:
        class: stdClass
        factory_service: bar
        factory_method: getValue
        arguments: [ 'bar' ] # arguments for free! ie: no PropertyAccess uage and its limitations

    foo:
        class: Foo
        arguments: [@bar_value]

Maybe I'm wrong, and nevertheless longer than the property path approach.

Contributor

docteurklein commented Aug 26, 2013

Isn't this functionality a shortcut to something already doable via service factory ?

services:
    bar:
        class: Bar

    bar_value:
        class: stdClass
        factory_service: bar
        factory_method: getValue
        arguments: [ 'bar' ] # arguments for free! ie: no PropertyAccess uage and its limitations

    foo:
        class: Foo
        arguments: [@bar_value]

Maybe I'm wrong, and nevertheless longer than the property path approach.

@gnutix

This comment has been minimized.

Show comment Hide comment
@gnutix

gnutix Aug 26, 2013

Contributor

Big +1 on this PR (see #7352). As @Crell stated, it would really add a lot of flexibility (and use cases) if we could improve the PropertyAccess component to support methods calls arguments, otherwise we may still be forced to use a similar approach than @docteurklein example in case we need arguments. Seems like a must have ! @bschussek What do you think would be the amount of work for that to happen? I may be able to get some time to work on it.

Concerning the Validator syntax consistency issue, I agree it would be a lot better to have only one syntax all over the framework. But I can't tell which one seems the best. Is the validator syntax already usable in Symfony ? Would it requires a BC break to change it ?

@docteurklein Yes it is, but the class: stdClass is a "non supported hack". See #7352.

Contributor

gnutix commented Aug 26, 2013

Big +1 on this PR (see #7352). As @Crell stated, it would really add a lot of flexibility (and use cases) if we could improve the PropertyAccess component to support methods calls arguments, otherwise we may still be forced to use a similar approach than @docteurklein example in case we need arguments. Seems like a must have ! @bschussek What do you think would be the amount of work for that to happen? I may be able to get some time to work on it.

Concerning the Validator syntax consistency issue, I agree it would be a lot better to have only one syntax all over the framework. But I can't tell which one seems the best. Is the validator syntax already usable in Symfony ? Would it requires a BC break to change it ?

@docteurklein Yes it is, but the class: stdClass is a "non supported hack". See #7352.

@docteurklein

This comment has been minimized.

Show comment Hide comment
@docteurklein

docteurklein Aug 26, 2013

Contributor

Thanks for pointing this issue out @gnutix.
However, this is a hack only if the factory service returns scalars.
If it's about naming in #7352, we could imagine a parameter_factory type of service (which is the exact same implementation as service_factory. Mmh.

Anyway, I agree it would be great for flexibility and simplification of current implementations to bypass this limitation (as @Crell shown above).

Contributor

docteurklein commented Aug 26, 2013

Thanks for pointing this issue out @gnutix.
However, this is a hack only if the factory service returns scalars.
If it's about naming in #7352, we could imagine a parameter_factory type of service (which is the exact same implementation as service_factory. Mmh.

Anyway, I agree it would be great for flexibility and simplification of current implementations to bypass this limitation (as @Crell shown above).

@hason

This comment has been minimized.

Show comment Hide comment
@hason

hason Aug 26, 2013

Contributor

The most popular framework in the Czech republic (nette/nette) has a much more expressive possibilities http://doc.nette.org/en/configuring#toc-configuration-using-neon-files (very similar to the example by @Crell )

services:
    database:
        class: Nette\Database\Connection(%dsn%, %user%, %password%)
        setup:
            - setCacheStorage(@factory::createStorage(%parameter%) )
Contributor

hason commented Aug 26, 2013

The most popular framework in the Czech republic (nette/nette) has a much more expressive possibilities http://doc.nette.org/en/configuring#toc-configuration-using-neon-files (very similar to the example by @Crell )

services:
    database:
        class: Nette\Database\Connection(%dsn%, %user%, %password%)
        setup:
            - setCacheStorage(@factory::createStorage(%parameter%) )
@Crell

This comment has been minimized.

Show comment Hide comment
@Crell

Crell Sep 1, 2013

Contributor

So that's a couple of votes yes... @fabpot, is adding parameter support at all feasible? (I don't know the internals of the DIC compiler at all.)

Contributor

Crell commented Sep 1, 2013

So that's a couple of votes yes... @fabpot, is adding parameter support at all feasible? (I don't know the internals of the DIC compiler at all.)

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Sep 1, 2013

Contributor

btw the topic of expressions in config is related to this discussion willdurand/Hateoas#54 .. then again maybe this is more flexibility than we want to offer.

Contributor

lsmith77 commented Sep 1, 2013

btw the topic of expressions in config is related to this discussion willdurand/Hateoas#54 .. then again maybe this is more flexibility than we want to offer.

@sstok

This comment has been minimized.

Show comment Hide comment
@sstok

sstok Sep 1, 2013

Contributor

For the PropertyAccess component to support parameter call I think of something like the Expressions used by the JMSSecurityExtraBundle or just giving it an array of parameter and letting the Reflection find there correct position (as reflection is already used right?).

The Spring version also supports things like comparisons and mathematics, should these be considered as well?
Only thing is how much do you want to support as the more complexity is introduced the harder its gets to use the container when not dumped to PHP code, you're only option would be using eval() then.

Just some thoughts.

Contributor

sstok commented Sep 1, 2013

For the PropertyAccess component to support parameter call I think of something like the Expressions used by the JMSSecurityExtraBundle or just giving it an array of parameter and letting the Reflection find there correct position (as reflection is already used right?).

The Spring version also supports things like comparisons and mathematics, should these be considered as well?
Only thing is how much do you want to support as the more complexity is introduced the harder its gets to use the container when not dumped to PHP code, you're only option would be using eval() then.

Just some thoughts.

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Sep 1, 2013

Contributor

We have in the past consciously decided to not support everything that Spring can. f.e. we dropped interface injection because that meant we would also have to add a syntax to override the interface injections.

Contributor

lsmith77 commented Sep 1, 2013

We have in the past consciously decided to not support everything that Spring can. f.e. we dropped interface injection because that meant we would also have to add a syntax to override the interface injections.

@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Sep 2, 2013

Member

This feature would make it possible to use https://github.com/opensky/OpenSkyRuntimeConfigBundle the right way instead of abusing services defined by a factory by putting a scalar into the service (which technically break the interface as the container as get($id) is expected to return an object).
However, to make it work with the current version of the bundle, it would also require passing arguments, like in the Drupal case. I guess it could work with PropertyAccess by implementing ArrayAccess or __get and __set to access the properties, but supporting arguments would make it easier.

@bschussek Do you have an idea of how much work this would represent to add it to the PropertyAccess component ? I don't really see how we could add arguments to it as it can access deep paths (and we would need to put the arguments in any level of this path). Maybe we could add support only for ParameterBag-style getters (i.e. get('foo') to access the value for foo) with a special syntax for it (for instance bag.<foo> ? It would be consistent with the way the property path is build for ArrayAccess. This would solve this for the case of Drupal and OpenSkyRuntimeConfigBundle at least.

Member

stof commented Sep 2, 2013

This feature would make it possible to use https://github.com/opensky/OpenSkyRuntimeConfigBundle the right way instead of abusing services defined by a factory by putting a scalar into the service (which technically break the interface as the container as get($id) is expected to return an object).
However, to make it work with the current version of the bundle, it would also require passing arguments, like in the Drupal case. I guess it could work with PropertyAccess by implementing ArrayAccess or __get and __set to access the properties, but supporting arguments would make it easier.

@bschussek Do you have an idea of how much work this would represent to add it to the PropertyAccess component ? I don't really see how we could add arguments to it as it can access deep paths (and we would need to put the arguments in any level of this path). Maybe we could add support only for ParameterBag-style getters (i.e. get('foo') to access the value for foo) with a special syntax for it (for instance bag.<foo> ? It would be consistent with the way the property path is build for ArrayAccess. This would solve this for the case of Drupal and OpenSkyRuntimeConfigBundle at least.

@@ -1197,6 +1212,8 @@ private function dumpValue($value, $interpolate = true)
}
return $this->getServiceCall((string) $value, $value);
+ } elseif ($value instanceof Expression) {
+ return sprintf("\$this->getPropertyAccessor()->getValue(%s, '%s')", $this->dumpValue(new Reference($value->getId())), $value->getPath());

This comment has been minimized.

Show comment Hide comment
@stof

stof Sep 2, 2013

Member

you need to escape quotes in the path to be safe (as you don't have any validation of the path before dumping). Otherwise, it could create some fatal error in the dumped code. I suggest using var_export to build the dumped string instead of just adding quotes around it

@stof

stof Sep 2, 2013

Member

you need to escape quotes in the path to be safe (as you don't have any validation of the path before dumping). Otherwise, it could create some fatal error in the dumped code. I suggest using var_export to build the dumped string instead of just adding quotes around it

@fabpot fabpot referenced this pull request Sep 2, 2013

Merged

New Component: Expression Language #8913

7 of 8 tasks complete
@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Sep 2, 2013

Owner

Closing in favor of #8913. The new PR takes a new approach where features asked in the comments are possible. Feedback welcome.

Owner

fabpot commented Sep 2, 2013

Closing in favor of #8913. The new PR takes a new approach where features asked in the comments are possible. Feedback welcome.

@fabpot fabpot closed this Sep 2, 2013

fabpot added a commit that referenced this pull request Sep 19, 2013

merged branch fabpot/expression-engine (PR #8913)
This PR was merged into the master branch.

Discussion
----------

New Component: Expression Language

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #8850, #7352
| License       | MIT
| Doc PR        | not yet

TODO:

 - [ ] write documentation
 - [x] add tests for the new component
 - [x] implement expression support for access rules in the security component
 - [x] find a better character/convention for expressions in the YAML format
 - [x] check the performance of the evaluation mode
 - [x] better error messages in the evaluation mode
 - [x] add support in the Routing
 - [x] add support in the Validator

The ExpressionLanguage component provides an engine that can compile and
evaluate expressions.

An expression is a one-liner that returns a value (mostly, but not limited to, Booleans).

It is a strip-down version of Twig (only the expression part of it is
implemented.) Like Twig, the expression is lexed, parsed, and
compiled/evaluated. So, it is immune to external injections by design.

If we compare it to Twig, here are the main big differences:

 * only support for Twig expressions
 * no ambiguity for calls (foo.bar is only valid for properties, foo['bar'] is only valid for array calls, and foo.bar() is required for method calls)
 * no support for naming conventions in method calls (if the method is named getFoo(), you must use getFoo() and not foo())
 * no notion of a line for errors, but a cursor (we are mostly talking about one-liners here)
 * removed everything specific to the templating engine (like output escaping or filters)
 * no support for named arguments in method calls
 * only one extension point with functions (no possibility to define new operators, ...)
 * and probably even more I don't remember right now
 * there is no need for a runtime environment, the compiled PHP string is self-sufficient

An open question is whether we keep the difference betweens arrays and hashes.

The other big difference with Twig is that it can work in two modes (possible
because of the restrictions described above):

 * compilation: the expression is compiled to PHP and is self-sufficient
 * evaluation: the expression is evaluated without being compiled to PHP (the node tree produced by the parser can be serialized and evaluated afterwards -- so it can be saved on disk or in a database to speed up things when needed)

Let's see a simple example:

```php
$language = new ExpressionLanguage();

echo $language->evaluate('1 + 1');
// will echo 2

echo $language->compile('1 + 2');
// will echo "(1 + 2)"
```

The language supports:

 * all basic math operators (with precedence rules):
    * unary: not, !, -, +
    * binary: or, ||, and, &&, b-or, b-xor, b-and, ==, ===, !=, !==, <, >, >=, <=, not in, in, .., +, -, ~, *, /, %, **

 * all literals supported by Twig: strings, numbers, arrays (`[1, 2]`), hashes
   (`{a: "b"}`), Booleans, and null.

 * simple variables (`foo`), array accesses (`foo[1]`), property accesses
   (`foo.bar`), and method calls (`foo.bar(1, 2)`).

 * the ternary operator: `true ? true : false` (and all the shortcuts
   implemented in Twig).

 * function calls (`constant('FOO')` -- `constant` is the only built-in
   functions).

 * and of course, any combination of the above.

The compilation is better for performances as the end result is just a plain PHP string without any runtime. For the evaluation, we need to tokenize, parse, and evaluate the nodes on the fly. This can be optimized by using a `ParsedExpression` or a `SerializedParsedExpression` instead:

```php
$nodes = $language->parse($expr, $names);
$expression = new SerializedParsedExpression($expr, serialize($nodes));

// You can now store the expression in a DB for later reuse

// a SerializedParsedExpression can be evaluated like any other expressions,
// but under the hood, the lexer and the parser won't be used at all, so it''s much faster.
$language->evaluate($expression);
```
That's all folks!

I can see many use cases for this new component, and we have two use cases in
Symfony that we can implement right away.

## Using Expressions in the Service Container

The first one is expression support in the service container (it would replace
#8850) -- anywhere you can pass an argument in the service container, you can
use an expression:

```php
$c->register('foo', 'Foo')->addArgument(new Expression('bar.getvalue()'));
```

You have access to the service container via `this`:

    container.get("bar").getvalue(container.getParameter("value"))

The implementation comes with two functions that simplifies expressions
(`service()` to get a service, and `parameter` to get a parameter value). The
previous example can be simplified to:

    service("bar").getvalue(parameter("value"))

Here is how to use it in XML:

```xml
<parameters>
    <parameter key="value">foobar</parameter>
</parameters>
<services>
    <service id="foo" class="Foo">
        <argument type="expression">service('bar').getvalue(parameter('value'))</argument>
    </service>
    <service id="bar" class="Bar" />
</services>
```

and in YAML (I chose the syntax randomly ;)):

```yaml
parameters:
    value: foobar

services:
    bar:
        class: Bar

    foo:
        class: Foo
        arguments: [@=service("bar").getvalue(parameter("value"))]
```

When using the container builder, Symfony uses the evaluator, but with the PHP
dumper, the compiler is used, and there is no overhead as the expression
engine is not needed at runtime. The expression above would be compiled to:

```php
$this->get("bar")->getvalue($this->getParameter("value"))
```

## Using Expression for Security Access Control Rules

The second use case in Symfony is for access rules.

As we all know, the way to configure the security access control rules is confusing, which might lead to insecure applications (see http://symfony.com/blog/security-access-control-documentation-issue for more information).

Here is how the new `allow_if` works:

```yaml
access_control:
    - { path: ^/_internal/secure, allow_if: "'127.0.0.1' == request.getClientIp() or has_role('ROLE_ADMIN')" }
```

This one restricts the URLs starting with `/_internal/secure` to people browsing from the localhost. Here, `request` is the current Request instance. In the expression, there is access to the following variables:

 * `request`
 * `token`
 * `user`

And to the following functions:

 * `is_anonymous`
 * `is_authenticated`
 * `is_fully_authenticated`
 * `is_rememberme`
 * `has_role`

You can also use expressions in Twig, which works well with the `is_granted` function:

```jinja
{% if is_granted(expression('has_role("FOO")')) %}
   ...
{% endif %}
```

## Using Expressions in the Routing

Out of the box, Symfony can only match an incoming request based on some pre-determined variables (like the path info, the method, the scheme, ...). But some people want to be able to match on more complex logic, based on other information of the Request object. That's why we introduced `RequestMatcherInterface` recently (but we no default implementation in Symfony itself).

The first change I've made (not related to expression support) is implement this interface for the default `UrlMatcher`. It was simple enough.

Then, I've added a new `condition` configuration for Route objects, which allow you to add any valid expression. An expression has access to the `request` and to the routing `context`.

Here is how one would configure it in a YAML file:

```yaml
hello:
    path: /hello/{name}
    condition: "context.getMethod() in ['GET', 'HEAD'] and request.headers.get('User-Agent') =~ '/firefox/i'"
```

Why do I keep the context as all the data are also available in the request? Because you can also use the condition without using the RequestMatcherInterface, in which case, you don't have access to the request. So, the previous example is equivalent to:

```yaml
hello:
    path: /hello/{name}
    condition: "request.getMethod() in ['GET', 'HEAD'] and request.headers.get('User-Agent') =~ '/firefox/i'"
```

When using the PHP dumper, there is no overhead as the condition is compiled. Here is how it looks like:

```php
// hello
if (0 === strpos($pathinfo, '/hello') && preg_match('#^/hello/(?P<name>[^/]++)$#s', $pathinfo, $matches) && (in_array($context->getMethod(), array(0 => "GET", 1 => "HEAD")) && preg_match("/firefox/i", $request->headers->get("User-Agent")))) {
    return $this->mergeDefaults(array_replace($matches, array('_route' => 'hello')), array ());
}
```

Be warned that conditions are not taken into account when generating a URL.

## Using Expressions in the Validator

There is a new Expression constraint that you can put on a class. The expression is then evaluated for validation:

```php
use Symfony\Component\Validator\Constraints as Assert;

/**
 * @Assert\Condition(condition="this.getFoo() == 'fo'", message="Not good!")
 */
class Obj
{
    public function getFoo()
    {
        return 'foo';
    }
}
```

In the expression, you get access to the current object via the `this` variable.

## Dynamic annotations

The expression language component is also very useful in annotations. the SensoLabs FrameworkExtraBundle leverages this possibility to implement HTTP validation caching in the `@Cache` annotation and to add a new `@Security` annotation (see sensiolabs/SensioFrameworkExtraBundle#238.)

Commits
-------

d4ebbfd [Validator] Renamed Condition to Expression and added possibility to set it onto properties
a3b3a78 [Validator] added a constraint that runs an expression
1bcfb40 added optimized versions of expressions
984bd38 mades things more consistent for the end user
d477f15 [Routing] added support for expression conditions in routes
86ac8d7 [ExpressionLanguage] improved performance
e369d14 added a Twig extension to create Expression instances
38b7fde added support for expression in control access rules
2777ac7 [HttpFoundation] added ExpressionRequestMatcher
c25abd9 [DependencyInjection] added support for expressions in the service container
3a41781 [ExpressionLanguage] added support for regexes
9d98fa2 [ExpressionLanguage] added the component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment