Permalink
Commits on Jan 9, 2012
  1. merged 2.0

    fabpot committed Jan 9, 2012
Commits on Jan 8, 2012
  1. Revert "merged 2.0"

    fabpot committed Jan 8, 2012
    This reverts commit 7000e944fd05446c75fdc35fd811005cbe2fa5c4, reversing
    changes made to 9d9013d662b311ae541fa98433441d330a83a869.
Commits on Jan 7, 2012
  1. [HttpFoundation] Fixed #3053

    stof committed Jan 7, 2012
Commits on Jan 5, 2012
  1. merged 2.0

    fabpot committed Jan 5, 2012
Commits on Jan 2, 2012
  1. [streaming] Do not set a Transfer-Encoding header of chunked

    igorw committed Jan 2, 2012
    Apache expects the response to already be in chunked format in that case,
    which causes it to not deliver the streamed body.
    
    If no Content-Length is set on the response, web servers will automatically
    switch to chunked Transfer-Encoding, and handle the chunking for you.
    
    Nginx does not share the issue that apache has, but will add the Content-
    Length header too.
Commits on Dec 31, 2011
  1. merged branch symfony/streaming (PR #2935)

    fabpot committed Dec 31, 2011
    Commits
    -------
    
    887c0e9 moved EngineInterface::stream() to a new StreamingEngineInterface to keep BC with 2.0
    473741b added the possibility to change a StreamedResponse callback after its creation
    8717d44 moved a test in the constructor
    e44b8ba made some cosmetic changes
    0038d1b [HttpFoundation] added support for streamed responses
    
    Discussion
    ----------
    
    [HttpFoundation] added support for streamed responses
    
    To stream a Response, use the StreamedResponse class instead of the
    standard Response class:
    
        $response = new StreamedResponse(function () {
            echo 'FOO';
        });
    
        $response = new StreamedResponse(function () {
            echo 'FOO';
        }, 200, array('Content-Type' => 'text/plain'));
    
    As you can see, a StreamedResponse instance takes a PHP callback instead of
    a string for the Response content. It's up to the developer to stream the
    response content from the callback with standard PHP functions like echo.
    You can also use flush() if needed.
    
    From a controller, do something like this:
    
        $twig = $this->get('templating');
    
        return new StreamedResponse(function () use ($templating) {
            $templating->stream('BlogBundle:Annot:streamed.html.twig');
        }, 200, array('Content-Type' => 'text/html'));
    
    If you are using the base controller, you can use the stream() method instead:
    
        return $this->stream('BlogBundle:Annot:streamed.html.twig');
    
    You can stream an existing file by using the PHP built-in readfile() function:
    
        new StreamedResponse(function () use ($file) {
            readfile($file);
        }, 200, array('Content-Type' => 'image/png');
    
    Read http://php.net/flush for more information about output buffering in PHP.
    
    Note that you should do your best to move all expensive operations to
    be "activated/evaluated/called" during template evaluation.
    
    Templates
    ---------
    
    If you are using Twig as a template engine, everything should work as
    usual, even if are using template inheritance!
    
    However, note that streaming is not supported for PHP templates. Support
    is impossible by design (as the layout is rendered after the main content).
    
    Exceptions
    ----------
    
    Exceptions thrown during rendering will be rendered as usual except that
    some content might have been rendered already.
    
    Limitations
    -----------
    
    As the getContent() method always returns false for streamed Responses, some
    event listeners won't work at all:
    
    * Web debug toolbar is not available for such Responses (but the profiler works fine);
    * ESI is not supported.
    
    Also note that streamed responses cannot benefit from HTTP caching for obvious
    reasons.
    
    ---------------------------------------------------------------------------
    
    by Seldaek at 2011/12/21 06:34:13 -0800
    
    Just an idea: what about exposing flush() to twig? Possibly in a way that it will not call it if the template is not streaming. That way you could always add a flush() after your </head> tag to make sure that goes out as fast as possible, but it wouldn't mess with non-streamed responses. Although it appears flush() doesn't affect output buffers, so I guess it doesn't need anything special.
    
    When you say "ESI is not supported.", that means only the AppCache right? I don't see why this would affect Varnish, but then again as far as I know Varnish will buffer if ESI is used so the benefit of streaming there is non-existent.
    
    ---------------------------------------------------------------------------
    
    by cordoval at 2011/12/21 08:04:21 -0800
    
    wonder what the use case is for streaming a response, very interesting.
    
    ---------------------------------------------------------------------------
    
    by johnkary at 2011/12/21 08:19:48 -0800
    
    @cordoval Common use cases are present fairly well by this RailsCast video: http://railscasts.com/episodes/266-http-streaming
    
    Essentially it allows faster fetching of web assets (JS, CSS, etc) located in the &lt;head>&lt;/head>, allowing those assets to be fetched as soon as possible before the remainder of the content body is computed and sent to the browser. The end goal is to improve page load speed.
    
    There are other uses cases too like making large body content available quickly to the service consuming it. Think if you were monitoring a live feed of JSON data of newest Twitter comments.
    
    ---------------------------------------------------------------------------
    
    by lsmith77 at 2011/12/21 08:54:35 -0800
    
    How does this relate the limitations mentioned in:
    http://yehudakatz.com/2010/09/07/automatic-flushing-the-rails-3-1-plan/
    
    Am I right to understand that due to how twig works we are not really streaming the content pieces when we call render(), but instead the entire template with its layout is rendered and only then will we flush? or does it mean that the render call will work its way to the top level layout template and form then on it can send the content until it hits another block, which it then first renders before it continues to send the data?
    
    ---------------------------------------------------------------------------
    
    by stof at 2011/12/21 09:02:53 -0800
    
    @lsmith77 this is why the ``stream`` method calls ``display`` in Twig instead of ``render``. ``display`` uses echo to print the output of the template line by line (and blocks are simply method calls in the middle). Look at your compiled templates to see it (the ``doDisplay`` method)
    Rendering a template with Twig simply use an output buffer around the rendering.
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/21 09:24:33 -0800
    
    @lsmith77: We don't have the Rails problem thanks to Twig as the order of execution is the right one by default (the layout is executed first); it means that we can have the flush feature without any change to how the core works. As @stof mentioned, we are using `display`, not `render`, so we are streaming your templates for byte one.
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/21 09:36:41 -0800
    
    @Seldaek: yes, I meant ESI with the PHP reverse proxy.
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/21 09:37:34 -0800
    
    @Seldaek: I have `flush()` support for Twig on my todo-list. As you mentioned, It should be trivial to implement.
    
    ---------------------------------------------------------------------------
    
    by fzaninotto at 2011/12/21 09:48:18 -0800
    
    How do streaming responses deal with assets that must be called in the head, but are declared in the body?
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/21 09:52:12 -0800
    
    @fzaninotto: What do you mean?
    
    With Twig, your layout is defined with blocks ("holes"). These blocks are overridden by child templates, but evaluated as they are encountered in the layout. So, everything works as expected.
    
    As noted in the commit message, this does not work with PHP templates for the problems mentioned in the Rails post (as the order of execution is not the right one -- the child template is first evaluated and then the layout).
    
    ---------------------------------------------------------------------------
    
    by fzaninotto at 2011/12/21 10:07:35 -0800
    
    I was referring to using Assetic. Not sure if this compiles to Twig the same way as javascript and stylesheet blocks placed in the head - and therefore executed in the right way.
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/21 10:34:59 -0800
    
    @Seldaek: I've just added a `flush` tag in Twig 1.5: fabpot/Twig@1d6dfad
    
    ---------------------------------------------------------------------------
    
    by catchamonkey at 2011/12/21 13:29:22 -0800
    
    I'm really happy you've got this into the core, it's a great feature to have! Good work.
Commits on Dec 26, 2011
  1. merged 2.0

    fabpot committed Dec 26, 2011
Commits on Dec 23, 2011
  1. fixed merge

    fabpot committed Dec 23, 2011
  2. merged 2.0

    fabpot committed Dec 23, 2011
Commits on Dec 22, 2011
Commits on Dec 21, 2011
  1. [HttpFoundation] fixed ApacheRequest

    kriswallsmith committed Dec 21, 2011
    Pathinfo was incorrect when using mod_rewrite.
    Added better test coverage.
  2. made some cosmetic changes

    fabpot committed Dec 21, 2011
  3. [HttpFoundation] added support for streamed responses

    fabpot committed Oct 17, 2011
    To stream a Response, use the StreamedResponse class instead of the
    standard Response class:
    
        $response = new StreamedResponse(function () {
            echo 'FOO';
        });
    
        $response = new StreamedResponse(function () {
            echo 'FOO';
        }, 200, array('Content-Type' => 'text/plain'));
    
    As you can see, a StreamedResponse instance takes a PHP callback instead of
    a string for the Response content. It's up to the developer to stream the
    response content from the callback with standard PHP functions like echo.
    You can also use flush() if needed.
    
    From a controller, do something like this:
    
        $twig = $this->get('templating');
    
        return new StreamedResponse(function () use ($templating) {
            $templating->stream('BlogBundle:Annot:streamed.html.twig');
        }, 200, array('Content-Type' => 'text/html'));
    
    If you are using the base controller, you can use the stream() method instead:
    
        return $this->stream('BlogBundle:Annot:streamed.html.twig');
    
    You can stream an existing file by using the PHP built-in readfile() function:
    
        new StreamedResponse(function () use ($file) {
            readfile($file);
        }, 200, array('Content-Type' => 'image/png');
    
    Read http://php.net/flush for more information about output buffering in PHP.
    
    Note that you should do your best to move all expensive operations to
    be "activated/evaluated/called" during template evaluation.
    
    Templates
    ---------
    
    If you are using Twig as a template engine, everything should work as
    usual, even if are using template inheritance!
    
    However, note that streaming is not supported for PHP templates. Support
    is impossible by design (as the layout is rendered after the main content).
    
    Exceptions
    ----------
    
    Exceptions thrown during rendering will be rendered as usual except that
    some content might have been rendered already.
    
    Limitations
    -----------
    
    As the getContent() method always returns false for streamed Responses, some
    event listeners won't work at all:
    
    * Web debug toolbar is not available for such Responses (but the profiler works fine);
    * ESI is not supported.
    
    Also note that streamed responses cannot benefit from HTTP caching for obvious
    reasons.
Commits on Dec 18, 2011
  1. merged 2.0

    fabpot committed Dec 18, 2011
  2. fixed CS

    fabpot committed Dec 18, 2011
  3. fixed CS

    fabpot committed Dec 18, 2011
  4. tweaked the README files

    fabpot committed Dec 18, 2011
  5. merged branch lsmith77/component_readmes (PR #2561)

    fabpot committed Dec 18, 2011
    Commits
    -------
    
    1e370d7 typo fix
    93d8d44 added some more infos about Config
    27efd59 added READMEs for the bridges
    34fc866 cosmetic tweaks
    d6af3f1 fixed README for Console
    6a72b8c added basic README files for all components
    
    Discussion
    ----------
    
    added basic README files for all components and bridges
    
    heavily based on http://fabien.potencier.org/article/49/what-is-symfony2 and the official Symfony2 documentation
    
    ---------------------------------------------------------------------------
    
    by jmikola at 2011/11/03 13:36:07 -0700
    
    Great work. For syntax highlighting on the PHP snippets, you could add "php" after the three backticks.
    
    ---------------------------------------------------------------------------
    
    by lsmith77 at 2011/11/03 13:41:29 -0700
    
    done
    
    ---------------------------------------------------------------------------
    
    by stealth35 at 2011/11/03 13:49:31 -0700
    
    Nice job, but you also need to add `<?php`
    
    ex :
    
    ``` php
    <?php
    use Symfony\Component\DomCrawler\Crawler;
    
    $crawler = new Crawler();
    $crawler->addContent('<html><body><p>Hello World!</p></body></html>');
    
    print $crawler->filter('body > p')->text();
    ```
    
    ---------------------------------------------------------------------------
    
    by lsmith77 at 2011/11/03 13:56:57 -0700
    
    done
    
    ---------------------------------------------------------------------------
    
    by ericclemmons at 2011/11/03 19:57:57 -0700
    
    @lsmith77 Well done!  This makes consumption of individual components that much easier, *especially* now that `composer.json` files have been added.
    
    ---------------------------------------------------------------------------
    
    by lsmith77 at 2011/11/04 01:18:23 -0700
    
    ok .. fixed the issues you mentioned @fabpot
    
    ---------------------------------------------------------------------------
    
    by lsmith77 at 2011/11/11 15:00:27 -0800
    
    @fabpot anything else left? seems like an easy merge .. and imho there is considerable benefit for our efforts to spread the word about the components with this PR merged.
    
    ---------------------------------------------------------------------------
    
    by drak at 2011/11/11 18:54:13 -0800
    
    You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
    
    ---------------------------------------------------------------------------
    
    by lsmith77 at 2011/11/12 00:59:14 -0800
    
    i did that in some. but i might have missed a few places.
    On 12.11.2011, at 03:54, Drak <reply@reply.github.com> wrote:
    
    > You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
    >
    > ---
    > Reply to this email directly or view it on GitHub:
    > symfony/symfony#2561 (comment)
    
    ---------------------------------------------------------------------------
    
    by breerly at 2011/11/21 10:28:36 -0800
    
    Pretty excited with this.
    
    ---------------------------------------------------------------------------
    
    by dbu at 2011/11/24 00:02:50 -0800
    
    is there anything we can help with to make this ready to be merged?
    
    ---------------------------------------------------------------------------
    
    by lsmith77 at 2011/12/18 02:39:23 -0800
    
    @fabpot: seriously .. if you are not going to deliver something "better" and don't provide a reason what is wrong with this .. then its beyond frustrating. i obviously do not claim that these README's are perfect (and certainly still no replacement for proper documentation), but I do claim that in their current form they are a radical step forward to potential users of the Symfony2 components.
Commits on Dec 14, 2011
  1. merged 2.0

    fabpot committed Dec 14, 2011
Commits on Dec 13, 2011
  1. merged branch pulzarraider/explode_optimalisation (PR #2782)

    fabpot committed Dec 13, 2011
    Commits
    -------
    
    cd24fb8 change explode's limit parameter based on known variable content
    b3cc270 minor optimalisations for explode
    
    Discussion
    ----------
    
    [FrameworkBundle][CssSelector][HttpFoundation][HttpKernel] [Security][Validator] Minor optimizations for "explode" function
    
    Bug fix: no
    Feature addition: no
    Backwards compatibility break: no
    Symfony2 tests pass: yes
    Fixes the following tickets: -
    Todo: -
    
    I added limit parameter in some places, where it may be usefull. I did not check the context of what values may have been exploded. So to not break anything, I added +1 to limit parameter.
    
    If you find out that in some places limit (or limit+1) is not important or meaningless, write a comment please and I will fix it.
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/07 06:56:49 -0800
    
    Adding +1 just to be sure to not break anything is clearly something we won't do. What is the benefit of doing that anyway?
    
    ---------------------------------------------------------------------------
    
    by pulzarraider at 2011/12/07 13:50:24 -0800
    
    The main idea of making this PR was to notify about some places that may run faster with just adding one parameter to explode function.
    
    If in code is someting like: ```list($a, $b) = explode(':', $s);```
    Function ```explode``` will create n-items (depends on ```$s```), but we need in code only the first two items. There is no reason to let ```explode``` create more items in memory that are NEVER used in our code. The limit parameter is there for these situations, so let's use it.
    
    I know that it is microoptimization and may look unimportant, but we are writing a framework - so people expect that code will be as fast as possible without this kind of mistakes.
    
    As I've noticed above, I know that +1 is not ideal solution, but the fastest without debugging the code. I expect that someone (with good knowledge of that code) will look at it and write in comments if variable may contain 1 comma (dot or someting on what is doing the explode) or maybe 2 in some situations or more.
    
    Anyway, +1 will not break anything, because same items are created as it is now, but no unnecessary item is created.
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/07 23:14:59 -0800
    
    I'm +1 for adding the number to avoid problems but I'm -1 on the optimization side of things as it won't optimize anything.
    
    ---------------------------------------------------------------------------
    
    by helmer at 2011/12/08 12:46:49 -0800
    
    *.. The main idea of making this PR was to notify about some places that **may** run faster ..*
    
    I am also unsure the optimization is really an optimization, care to benchmark (with meaningful inputs)? As for the limit+1 thing, why would you want to +1 it? The number of ``list`` arguments should always reflect the ``limit`` parameter, no?
    
    ---------------------------------------------------------------------------
    
    by pulzarraider at 2011/12/08 23:11:34 -0800
    
    @helmer please try this simple benchmark:
    
    ```
    <?php
    
    header('Content-Type: text/plain; charset=UTF-8');
    define('COUNT', 10000);
    
    $source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc:dddddddddddddddddddddd:eeeeeeeeeeeeeeeeeeeeeeeee:fffffffffffffffffffffffffff';
    
    $start = microtime(true);
    for ($i = 0; $i < COUNT; $i++) {
        list($a, $b) = explode(':', $source_string);
    }
    $end = microtime(true)-$start;
    echo 'without limit: '.$end."\n";
    
    $start = microtime(true);
    for ($i = 0; $i < COUNT; $i++) {
        list($a, $b) = explode(':', $source_string, 2);
    }
    $end = microtime(true)-$start;
    echo 'with limit:    '.$end."\n";
    ```
    
    My results are:
    
    ```
    without limit: 0.057228803634644
    with limit:    0.028676986694336
    ```
    That is 50% difference (with APC enabled).  Of course the result depends on the length of source string and if it's too short, the difference may be none or very very small. That's why I said, that it **may** run faster and is just a micro optimization.
    
    ---------------------------------------------------------------------------
    
    by pulzarraider at 2011/12/08 23:18:12 -0800
    
    @helmer And why +1? It depends on a code:
    
    ```
    $source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc';
    list($a, $b) = explode(':', $source_string, 2);
    var_dump($a, $b);
    ```
    
    and
    
    ```
    $source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc';
    list($a, $b) = explode(':', $source_string, 3);
    var_dump($a, $b);
    ```
    gives different results. That's why the content of the variable must be known.
    
    ---------------------------------------------------------------------------
    
    by helmer at 2011/12/09 00:08:28 -0800
    
    @pulzarraider Thanks for the benchmark, seems like a gain enough. Although, we are more likely having a scenario of:
    ``explode(':', 'a🅱️c')`` vs ``explode(':', 'a🅱️c', 3)`` with a ``COUNT`` of 10, where the difference is not even in microseconds anymore :)
    
    The limit addition alters the behaviour though, ie suddenly you can define a controller [logical name](http://symfony.com/doc/current/book/routing.html#controller-string-syntax) as ´´AcmeBlogBundle:Blog:show:something``, and things go downhill from there on.
    
    All that aside, I'm +1 for setting the limit to the exact number of ``list`` parameters, but certainly not number+1, this is just too wtfy (as you said, this was a safety thing, but I reckon for this PR to be merged it needs to be +0).
    
    ---------------------------------------------------------------------------
    
    by drak at 2011/12/09 08:28:58 -0800
    
    Overall `list()` is ugly as it's not very explicit.  Even though it would mean extra lines, it's better to `explode()` then explicitly assign variables:
    
    ```
    $parts = explode(':', $foo);
    $name = $parts[0];
    $tel = $parts[1];
    ```
    
    `list()` is one of those bad relics from the PHP past...
    
    ---------------------------------------------------------------------------
    
    by fabpot at 2011/12/11 10:07:47 -0800
    
    @drak: why is `list` not explicit? It is in fact as explicit as the more verbose syntax you propose.
    
    ---------------------------------------------------------------------------
    
    by pulzarraider at 2011/12/11 13:08:50 -0800
    
    @drak: I agree with @fabpot. In speech of benchmarks ```list``` is faster then using a helper variable.
    
    @fabpot, @helmer I've changed explode's limit to be correct (without +1) and removed some changes from this PR, where I can't find out what the content of variable may be. Unit tests pass, so I think it's ready for merge.
  2. merged 2.0

    fabpot committed Dec 13, 2011
  3. [DoctrineBridge] fixed some CS

    fabpot committed Dec 13, 2011
Commits on Dec 11, 2011
Commits on Dec 8, 2011
  1. merged 2.0

    fabpot committed Dec 8, 2011
Commits on Nov 23, 2011
  1. fixed bad merge

    fabpot committed Nov 23, 2011