Refactoring View Strategies

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Refactoring View Strategies

SocalNick
As many of you have noticed, a recent change in ZF2 caused the JsonStrategy to be chosen for all requests. This is because the strategies now use Accept header matching and all modern browsers send */*;q=0.8 as part of their accept header. Technically, if the JsonStrategy can render application/json and the browser says */* is ok, the JsonStrategy is a valid strategy.

Obviously, we normally want text/html in this default case. I've been working with Freeaqingme (Dolf Schimmel) over IRC the past few days trying to come up with the best solution. I have posted 3 PRs:

  1. https://github.com/zendframework/zf2/pull/1929
  2. https://github.com/zendframework/zf2/pull/1925
  3. https://github.com/zendframework/zf2/pull/1930

Please read the PR for full details, but I will summarize below.

The first was my attempt at recognizing that the match of */* was a low priority and we probably want to use the PhpRenderer in that case. However, this got weird as both the JsonStrategy and PhpRendererStrategy matched at 0.8 but we were choosing the PhpRendererStrategy even though it is registered  as the lowest priority strategy. It also was weird as the PhpRendererStrategy was no longer a very simple fallback case that simply returned a renderer.

The second was based on some pseudo code from Freeaqingme. The concept is based around a new strategy called AcceptHeaderStrategy that is actually a container for more strategies. This required adding a TextHtmlStrategy to match at higher priority than JsonStrategy and FeedStrategy. Unfortunately, this means that an AcceptHeaderStrategy is always matched and I can't register a strategy with lower priority than AcceptHeaderStrategy.

The third is a very simple fix that makes the JsonStrategy return a renderer only if the match was exactly application/json or application/javascript. This is identical to how the FeedStrategy works. This may not be correct according to RFC2616, but works very well for our real world use case.


It is my opinion that we should move forward with the 3rd fix for the near term. If the 1st and 2nd versions are really desired, they should probably be documented as an RFC and voted on at a meeting as they are much larger changes.


Thanks,

-Nick (@SocalNick)

Reply | Threaded
Open this post in threaded view
|

Re: Refactoring View Strategies

Steve Rhoades
+1 for #3.

I agree that it makes sense to have the default rendering strategy in this case (*/*) be the PhpRenderer and that applications requesting a different format (application/json .. text/xml) should indicate so in the Accept header.

- Steve

From: Nicholas Calugar <[hidden email]<mailto:[hidden email]>>
Date: Thursday, July 19, 2012 3:20 PM
To: Zend Framework Contributors <[hidden email]<mailto:[hidden email]>>
Subject: [zf-contributors] Refactoring View Strategies

As many of you have noticed, a recent change in ZF2 caused the JsonStrategy to be chosen for all requests. This is because the strategies now use Accept header matching and all modern browsers send */*;q=0.8 as part of their accept header. Technically, if the JsonStrategy can render application/json and the browser says */* is ok, the JsonStrategy is a valid strategy.

Obviously, we normally want text/html in this default case. I've been working with Freeaqingme (Dolf Schimmel) over IRC the past few days trying to come up with the best solution. I have posted 3 PRs:


  1.  https://github.com/zendframework/zf2/pull/1929
  2.  https://github.com/zendframework/zf2/pull/1925
  3.  https://github.com/zendframework/zf2/pull/1930

Please read the PR for full details, but I will summarize below.

The first was my attempt at recognizing that the match of */* was a low priority and we probably want to use the PhpRenderer in that case. However, this got weird as both the JsonStrategy and PhpRendererStrategy matched at 0.8 but we were choosing the PhpRendererStrategy even though it is registered  as the lowest priority strategy. It also was weird as the PhpRendererStrategy was no longer a very simple fallback case that simply returned a renderer.

The second was based on some pseudo code from Freeaqingme. The concept is based around a new strategy called AcceptHeaderStrategy that is actually a container for more strategies. This required adding a TextHtmlStrategy to match at higher priority than JsonStrategy and FeedStrategy. Unfortunately, this means that an AcceptHeaderStrategy is always matched and I can't register a strategy with lower priority than AcceptHeaderStrategy.

The third is a very simple fix that makes the JsonStrategy return a renderer only if the match was exactly application/json or application/javascript. This is identical to how the FeedStrategy works. This may not be correct according to RFC2616, but works very well for our real world use case.


It is my opinion that we should move forward with the 3rd fix for the near term. If the 1st and 2nd versions are really desired, they should probably be documented as an RFC and voted on at a meeting as they are much larger changes.


Thanks,

-Nick (@SocalNick)
Reply | Threaded
Open this post in threaded view
|

Re: Refactoring View Strategies

Matus Zeman
Same opinion here... give #3 a go!

Matus

On Fri, Jul 20, 2012 at 12:31 AM, Steve Rhoades <[hidden email]> wrote:
+1 for #3.

I agree that it makes sense to have the default rendering strategy in this case (*/*) be the PhpRenderer and that applications requesting a different format (application/json .. text/xml) should indicate so in the Accept header.

- Steve

From: Nicholas Calugar <[hidden email]<mailto:[hidden email]>>
Date: Thursday, July 19, 2012 3:20 PM
To: Zend Framework Contributors <[hidden email]<mailto:[hidden email]>>
Subject: [zf-contributors] Refactoring View Strategies

As many of you have noticed, a recent change in ZF2 caused the JsonStrategy to be chosen for all requests. This is because the strategies now use Accept header matching and all modern browsers send */*;q=0.8 as part of their accept header. Technically, if the JsonStrategy can render application/json and the browser says */* is ok, the JsonStrategy is a valid strategy.

Obviously, we normally want text/html in this default case. I've been working with Freeaqingme (Dolf Schimmel) over IRC the past few days trying to come up with the best solution. I have posted 3 PRs:


  1.  https://github.com/zendframework/zf2/pull/1929
  2.  https://github.com/zendframework/zf2/pull/1925
  3.  https://github.com/zendframework/zf2/pull/1930

Please read the PR for full details, but I will summarize below.

The first was my attempt at recognizing that the match of */* was a low priority and we probably want to use the PhpRenderer in that case. However, this got weird as both the JsonStrategy and PhpRendererStrategy matched at 0.8 but we were choosing the PhpRendererStrategy even though it is registered  as the lowest priority strategy. It also was weird as the PhpRendererStrategy was no longer a very simple fallback case that simply returned a renderer.

The second was based on some pseudo code from Freeaqingme. The concept is based around a new strategy called AcceptHeaderStrategy that is actually a container for more strategies. This required adding a TextHtmlStrategy to match at higher priority than JsonStrategy and FeedStrategy. Unfortunately, this means that an AcceptHeaderStrategy is always matched and I can't register a strategy with lower priority than AcceptHeaderStrategy.

The third is a very simple fix that makes the JsonStrategy return a renderer only if the match was exactly application/json or application/javascript. This is identical to how the FeedStrategy works. This may not be correct according to RFC2616, but works very well for our real world use case.


It is my opinion that we should move forward with the 3rd fix for the near term. If the 1st and 2nd versions are really desired, they should probably be documented as an RFC and voted on at a meeting as they are much larger changes.


Thanks,

-Nick (@SocalNick)

Reply | Threaded
Open this post in threaded view
|

Re: Refactoring View Strategies

richard
This post has NOT been accepted by the mailing list yet.
In reply to this post by SocalNick
I vote for letting the developer decide which is the most correct strategy.

It does not make sense to decide which proposal is correct for all cases.

I'd suggest that #3 is a reasonable default, with #2 as the means to configure behavior.