[MAGNOLIA-1698] Review usage of originalURI/currentURI, forward/include, virtualUriFilter Created: 23/Aug/07 Updated: 23/Jan/13 Resolved: 01/Nov/07 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core |
| Affects Version/s: | 3.5 RC1 |
| Fix Version/s: | 3.5 RC1 |
| Type: | Task | Priority: | Major |
| Reporter: | Magnolia International | Assignee: | Fabrizio Giustina |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||
| Task DoR: |
Empty
|
||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||
| Description |
|
We should review
|
| Comments |
| Comment by Magnolia International [ 24/Aug/07 ] |
|
just for the record, here are a couple of change Philipp and I tried out - ! not be applied as is without further thinking and refining |
| Comment by Philipp Bracher [ 27/Aug/07 ] |
|
Would like to explain that with an example to show why I consider that as very important topic. Assume you implement a controller (using a magnolia page, servlet or other framework). Now after executing your controllers code you want to forward to a magnolia page (/forum/register-user.html for example) This fails because the filter chain is not re-executed and the cms rendering won't work ( you end up in a 404). If the configuration of the filer would be defined by using <dispatcher>FORWARD</dispatcher> the filter would be re-executed on a forward. But after changing that we run into problems when we call a page directly (404). This happens because the JspTemplateRenderer uses a forward instead of an include what triggers a double execution of the filter also for single page calls. To avoid that we change JspTemplateRenderer so that it uses an include (now a normal page is rendered nicely again). Has anyone concerns about changing that (using include instead of a forward)? If we change that we have to change the the virtual uri mapping filter as well, since it does some 'workarounds' to re-execute the filter chain itself. The original uri is then set when the filter is executed (also when you do a forward) the current uri needs to be renamed to something like 'resolved uri' because this is the uri resolved by the virtual uri mappings. The resolved uri is used to read the right content node, the original uri is used to create the cache key. I'd like to do that rather sooner than later since this facilitates the integration of other frameworks. Since the magniolia filter chain uses bypasses you can avoid the re-execution by defining an other bypass, but you can not solve the described issue without having doing the changes we talked about. So I vote for changing that immediately. |
| Comment by Philipp Bracher [ 02/Oct/07 ] |
|
it's part of the 3.1 release because we have already done changes/cleanup in the filter execution and aggregation handling. This tasks seams to be very much related and increases the integration with other systems. |
| Comment by Fabrizio Giustina [ 03/Oct/07 ] |
|
I agree that this should be changed asap... this could require some configuration change for people using magnolia together with a mvc framework, but it will also solve tons of problems. |
| Comment by Philipp Bracher [ 05/Oct/07 ] |
|
We should not forget to make the filters configurable in a way that one can define each filters behavior. Actually we had some internal discussions about releasing 3.1 as soon as possible and doing a 3.2 release very soon afterward (february). We would then add only a few things like this issue to the 3.2. So that we solve only the critical issues now. But no final decision yet. |
| Comment by Fabrizio Giustina [ 27/Oct/07 ] |
|
Not sure when we want to make this jump, but after the latest changes already in svn for 3.1 this looks easier now 3 steps:
I have tested this for a couple of weeks and I was happily able to use forwards and error pages within magnolia, give it a try |
| Comment by Philipp Bracher [ 31/Oct/07 ] |
|
I think you can commit. My arguments for a commiting now are:
Can you first add a conditional task which checks that the dispatching is added in web.xml Can you also try to do a single commit, so that reverting is easy (we lack the time of chasing down things) |
| Comment by Fabrizio Giustina [ 01/Nov/07 ] |
|
Committed to svn, now the magnolia filter works properly with forward and error dispatchers. A new update task check for the existence of the dispatcher element in web.xml |