DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-03-10 08:22 ---
(In reply to comment #21)
> (In reply to comment #20)
> > I can't see a different way to get this done without changing much of Tiles/
> > Struts integration.
> > And I'm not sure it's worth the effort since the problem should disappear 
> > as 
> > from release 1.3.x.
> 
> OK so the conclusion we're agreed on is that it isn't going to get fixed in 
> 1.2.x - so closing this as WONTFIX.

we don't but that's your decision at the end of the day - you rule - so I just 
accept it.

Anyway, since the issue described by this ticket - leaving the patch alone - 
actually underlines a disfunctioning (call it limitation) of Struts-Tiles 
integration in the 1.1 and 1.2 series the least we can do, for honesty' sake, 
is 
to publish a few lines about it somewhere.

Since I believe the 1.2 series will be alive in Production environments 
worldwide for quite a long time yet that could avoid someone to lose time again 
on the same problem.

I can put it in the Wiki but I'm not sure it's the best place for it.
If you can think of any better place (a "limitations" section or anything 
similar) let me know.

Tx for your time anyway
Patrick

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703


[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||WONTFIX




--- Additional Comments From [EMAIL PROTECTED]  2006-03-07 15:49 ---
(In reply to comment #20)
> I can't see a different way to get this done without changing much of Tiles/
> Struts integration.
> And I'm not sure it's worth the effort since the problem should disappear as 
> from release 1.3.x.

OK so the conclusion we're agreed on is that it isn't going to get fixed in 
1.2.x - so closing this as WONTFIX.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-02-20 17:06 ---
(In reply to comment #19)
> Part 2 of your patch (TilesUtilStrutsImpl) still calls TilesRequestProcessor. 
>  
That's the part I have a big 
> problem with.  I'd have no problem if TilesRequestProcessor called TilesUtil, 
but I don't want the util class 
> to call the RequestProcessor.  I acknowledge that you have come across 
something that doesn't work for 
> your case in the existing 1.2.x version of the framework.  But I respectfully 
disagree with the solution 
> you've implemented.  If I think of a better way I'll post it.

ok, tx for looking into it.
I can't see a different way to get this done without changing much of Tiles/
Struts integration.
And I'm not sure it's worth the effort since the problem should disappear as 
from release 1.3.x.

Without changing anything this would mean that "routing" logic already in 
RequestProcessor's doInclude() has 
- either to be duplicated in subclasses of TilesUtilStrutsImpl and/or 
TilesUtilStrutsModuleImpl 
- or, to avoid code/logic duplication, taken out in some other utility class 
which all interested classes (including TilesRequestProcessor, 
TilesUtilStrutsImpl and/or TilesUtilStrutsModuleImpl) would have to rely upon 
...
but in either case that would lead Tiles and Struts integration not to be 
transparent any more, so both these solutions seem worse to me than a simple 
hook back to TilesRequestProcessor.

Anyway if you come up with a more suitable solution you can count on me to give 
a hand for both testing or developing it.

tx
Patrick

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-02-20 15:55 ---
Part 2 of your patch (TilesUtilStrutsImpl) still calls TilesRequestProcessor.  
That's the part I have a big 
problem with.  I'd have no problem if TilesRequestProcessor called TilesUtil, 
but I don't want the util class 
to call the RequestProcessor.  I acknowledge that you have come across 
something that doesn't work for 
your case in the existing 1.2.x version of the framework.  But I respectfully 
disagree with the solution 
you've implemented.  If I think of a better way I'll post it.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-02-19 19:20 ---
(In reply to comment #17)
> I appreciate that you've taken the trouble to do the patch and provide a 
> sample webapp, but I am still against your proposal.

No problem, glad to bring my 2 cents, especially for something that I believe 
is 
worth it.

> As I said in comment #5 IMO these tiles "util" classes should not call the 
> RequestProcessor.

Remember that comment #5 was regarding putting the fix in TilesUtil which was 
wrong - I agreed with that. And this was fixed in a second release of the patch 
(see comment #6).

> Also as I said in comment #5 you can override the tiles util implementation 
> used to provide your own custom processing to resolve this.

It just won't work because of a broken link in current Tiles/Struts integration.
This is documented in Test Phase 1 in comment #16.
TilesRequestProcessor would need to be patched (first part of the proposed 
patch) for that to work.

Re-assuming it in a few words: 
take a Struts web application using a custom RequestProcessor which overrides 
doInclude() and bring Tiles into it. Then, your implementation of doInclude() 
will never get called any more for any of your tiles.
Is that ok with you ?
Well it isn't with me.

> I still think we should close this as WONTFIX.
Sorry for being such a pain keeping on insisting on that one, but since this is 
a blocking issue in current Tiles/Struts integration WONTFIX isn't an answer.
If we come up with a better solution than the proposed patch that's fine for 
me. 
But in any case we do need to come up with a proper solution and I'm not giving 
up until we do.

Tx 
Patrick

P.S.: to be honest I still don't understand why you don't like the idea of 
putting the fix in TilesUtilStrutsImpl and TilesUtilStrutsModuleImpl as a 
default behaviour, everyone being free to change this default behaviour by 
overriding these classes, rather than forcing everyone to implement something 
to 
get this to work!

The above 2 classes are already there to make the glue between Tiles and 
Struts, 
as a default implementation, so I don't see the point of not using them for 
what 
they were originally meant for (again look at the Javadoc comments in 
TilesUtilStrutsModuleImpl).


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-02-12 01:17 ---
(In reply to comment #15)
> There's still a huge misunderstanding here.

>From my point of view there isn't a misunderstanding - I just disagree with 
you. I appreciate that you've taken the trouble to do the patch and provide a 
sample webapp, but I am still against your proposal.

As I said in comment #5 IMO these tiles "util" classes should not call the 
RequestProcessor.

Also as I said in comment #5 you can override the tiles util implementation 
used to provide your own custom processing to resolve this.

I still think we should close this as WONTFIX.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-02-05 18:53 ---
Created an attachment (id=17596)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=17596&action=view)
simple Web App to demonstrate the issue raised by this bug report and the
purpose of the patch proposed

Attached is a simple web app to demonstrate both the issue raised by this bug
report and the purpose of the patch proposed.

You just need to unzip the webapp under your favorite workspace directory and
have Tomcat point at it or deploy it under Tomcat directly it's as you prefer.

There are 2 test pages:
- test.jsp in jsp/notiles does not use Tiles (1)
- test.jsp in jsp/tiles uses Tiles (2)

(1) is called through a simple forward action:
 http://localhost:8080/test_bug35703/test_notiles.do 
(2) can be called either through a simple forward action or through Tiles
definitions + a forward in Struts config:
 http://localhost:8080/test_bug35703/test_tiles.do 
 http://localhost:8080/test_bug35703/test_tiles_using_defs.do

The RequestProcessor subclass dummy implementation simply overrides doForward()
and doInclude().
In both of them it prints out some system out and then calls the parent class
method.

Test Phase 1

Using Struts library release 1.2.x (I used 1.2.7 but you can use any 1.2.x
release or 1.1 as well, although you may have to fix struts-config, well at
least the system id).

a) call http://localhost:8080/test_bug35703/test_notiles.do 
Console: 
DemoRequestProcessor::doForward() called for uri=/jsp/tiles/layout/layout.jsp

b) call http://localhost:8080/test_bug35703/test_tiles.do 
Console: 
DemoRequestProcessor::doForward() called for uri=/jsp/tiles/test.jsp

c) call http://localhost:8080/test_bug35703/test_tiles_using_defs.do 
Console: 
DemoRequestProcessor::doForward() called for uri=/jsp/tiles/layout/layout.jsp


Test Phase 2

Using Struts library release 1.2.x patched (with proposed patch)

a) call http://localhost:8080/test_bug35703/test_notiles.do 
Console: 
DemoRequestProcessor::doForward() called for uri=/jsp/tiles/layout/layout.jsp

b) call http://localhost:8080/test_bug35703/test_tiles.do 
Console: 
DemoRequestProcessor::doForward() called for uri=/jsp/tiles/test.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/layout/layout.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/common/header.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/common/menu.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/test/body_test.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/common/footer.jsp

c) call http://localhost:8080/test_bug35703/test_tiles_using_defs.do 
Console: 
DemoRequestProcessor::doForward() called for uri=/jsp/tiles/layout/layout.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/common/header.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/common/menu.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/test/body_test.jsp
DemoRequestProcessor::doInclude() called for uri=/jsp/tiles/common/footer.jsp

Sumary
--
With the Struts 1.2.x library - see Test Phase 1 above - the contract of the
RequestProcessor gets broken (it doesn't get called for any of the tiles).
Whereas with the patched library  - see Test Phase 2 above - the
RequestProcessor doInclude() is being called for EVERY SINGLE tile.

HTH
Patrick


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-29 16:11 ---
There's still a huge misunderstanding here.
This bug report identifies an issue with current Tiles integration with Struts 
up to 1.2.x.
Anyway, I decided to provide a simple web app example in the next few days to 
help you guys really understand that.
I believe it's always worth it when there's a malfunction somewhere to insist 
on 
tackling the issue.

Anyway we should stop discussing Tiles standalone components here, which is 
irrelevant, but keep our minds focussed on the components of Tiles responsible 
for integration with Struts i.e. Tiles plugin.

AFAIK in current Tiles plugin architecture there are 3 components which 
actually 
cause the issue described in this bug report:
- TilesRequestProcessor (extends RequestProcessor)
- TilesUtilStrutsImpl (extends TilesUtilImpl)
- TilesUtilStrutsModuleImpl (extends TilesUtilStrutsImpl)

The reason for that being that all calls to doForward() and doInclude() methods 
end up in the above 3 components + TilesUtilImpl (used for Tiles standalone).
TilesUtilStrutsImpl and TilesUtilStrutsModuleImpl are the substitutes for 
TilesUtilImpl when it comes to Tiles + Struts integration.
And that's where the contract of the RequestProcessor gets broken hence this 
bug 
report.

It then seems as the natural places to issue the actual calls to 
TilesRequestProcessor, exactly where the person who designed 
TilesUtilStrutsModuleImpl and TilesUtilStrutsModuleImpl classes in the first 
place already planned them to be (again see comments in 
TilesUtilStrutsModuleImpl header).

Now if you want to re-design Tiles + Struts integration components that's a 
completely different matter - and I've got no objection to that. 
But that's got nothing to do with current issue which affects Tiles integration 
with existing releases of Struts up to 1.2.x.
And this patch merely aims at fixing the issue in that context, no more.


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-27 18:04 ---
(In reply to comment #7)
> Agree with Niall's comments on updating the patch.
> I finally add some time to work on it (sorry for the delay).

My comment #5 was this should be closed as WONTFIX or INVALID!

(In reply to comment #13)
I agree with what Greg has said in comment #11 and comment #13.

As Greg said, perhaps we could modify the RequestProcessor to call TilesUtil 
(haven't looked at the code recently). The question though is "is it worth 
it"? From what I see all it would do is save you having to duplicate your 
custom code in two places - that doesn't seem too much of a hardship to me. 
With the plan to develop "standalone tiles" which should replace this its 
probably not worth the effort IMO and we should just close this as WONTFIX.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-27 17:39 ---
(In reply to comment #12)
> Why does it break the contract of the RequestProcessor then would you ask ?
> When it comes to Tiles (until version 1.2.x of Struts), i.e. integrating 
> Tiles 
> with Struts via Tiles Plugin, the application-specific TilesRequestProcessor 
> implementation is actually bypassed. 

You mean your TilesRequestProcessor.doInclude() is not called?  And this is 
because 
TilesRequestProcessor does not override doInclude()?

If that's the case I do not have a problem with modifying TilesRequestProcessor 
to override doInclude() 
and call super.doInclude().  But I still don't like the idea of a class calling 
a RequestProcessor hook 
directly.  As Niall said before, those hooks are meant to be invoked in a chain 
by RequestProcessor, not 
individually by other components.  It would be my preference if 
TilesRequestProcessor's doInclude() 
called a method in TilesUtil that could be overriden by application developers 
instead of TilesUtil calling 
TilesRequestProcessor.doInclude().



-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-27 10:51 ---
I actually did address Niall's concerns.
On the contrary, current implementation of Tiles Plugin (until version 1.2.x of 
Struts) breaks the contract of the RequestProcessor - and this can be 
demonstrated pretty easily.
The patch proposed is meant to fix that.

Why does it break the contract of the RequestProcessor then would you ask ?
Each application is able to implement its own RequestProcessor(s), as you know, 
to satisfy its own specific requirements (and I just happen to have one which 
implements its own doInclude() and doForward() methods to face some 
multi-portal 
requirements, hence this bug report :-(, otherwise I probably wouldn't even 
have 
noticed the issue).
In the case of Struts without Tiles there's no problem at all, it all works 
fine.
When it comes to Tiles (until version 1.2.x of Struts), i.e. integrating Tiles 
with Struts via Tiles Plugin, the application-specific TilesRequestProcessor 
implementation is actually bypassed. 
This is certainly fine when you have Tiles standalone but it is not when it 
comes to Struts+Tiles integration.
As a matter of fact any application implementing its own TilesRequestProcessor 
subclass (with specific doForward() and doInclude() implementations) does not 
work any more with current Tiles Plugin (until version 1.2.x of Struts).

That's why the fix has been made at 2 levels which you may note now cares not 
to 
impact Tiles standalone (TilesUtilImpl is left as is - Niall's was correct 
about 
my first patch being too radical) and which now impacts Tiles integration with 
Struts only :
- TilesRequestProcessor now explicitly call parent's doInclude() (exactly in 
the 
same way that doForward() does)
- TilesUtilStrutsImpl and TilesUtilStrutsModuleImpl now rely on 
TilesRequestProcessor doInclude() and doForward()

N.B.: making the patch I actually noticed that the person who wrote 
TilesUtilStrutsModuleImpl in the first place actually realized this had to be 
done, just look at the class header comments and you'll find :
"Methods doForward() and doInclude() use their counterparts in the current 
RequestProcessor (todo)"


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-26 19:05 ---
I still don't think you've addressed Niall's concern about breaking the 
contract of the RequestProcessor, 
especially with regards to code in doInclude() that relies on something that 
happens earlier in the 
RequestProcessor chain of events.  The RequestProcessor is meant to be executed 
as a whole.  The hooks 
are plugin points for applications to override specific steps in the process.  
IMO, it's not a good idea to 
start using that API in other parts of the framework.  

To me part 1 of your patch is a workaround to expose part of the API that was 
not meant to be exposed.  I 
would personally be very hestant to stretch the 1.2.x API in this manner.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-26 17:31 ---
All patches, originally designed for version 1.2.7 of Struts, can be applied 
safely to version 1.2.8 as well as the impacted classes (TilesRequestProcessor, 
TilesUtilStrutsImpl and TilesUtilStrutsModuleImpl) didn't change between the 2 
releases.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-26 17:27 ---
Created an attachment (id=17511)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=17511&action=view)
third part of the patch (TilesUtilStrutsModuleImpl)

third part of the patch (TilesUtilStrutsModuleImpl)

Removed doInclude() and doForward() methods so it makes use of parent class
(TilesUtilStrutsImpl) implementation (which in turn calls TilesRequestProcessor
doInclude() and doForward() methods, see patch' second part).

The differentiation is made through a different implementation of
getRequestProcessor() for module-aware applications.


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-26 17:21 ---
Created an attachment (id=17510)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=17510&action=view)
second part of the patch (TilesUtilStrutsImpl)

1) doInclude() and doForward() methods rely on TilesRequestProcessor
corresponding methods
2) Added getRequestProcessor() 

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2006-01-26 17:19 ---
Agree with Niall's comments on updating the patch.
I finally add some time to work on it (sorry for the delay).

It's now made of 3 parts:
- first part remains identical (patch to TilesRequestProcessor already 
submitted) to allow TilesRequestProcessor doInclude() method to be called on 
TilesRequestProcessor subclasses (if any)
- second part is a patch for TilesUtilStrutsImpl to rely on the 
TilesRequestProcessor doInclude() and doForward() methods
- third part is for TilesUtilStrutsImpl (for module-aware applications) to rely 
on the TilesRequestProcessor doInclude() and doForward() methods

Patrick

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703


[EMAIL PROTECTED] changed:

   What|Removed |Added

  Attachment #15655|0   |1
is obsolete||




-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2005-11-08 10:30 ---
(In reply to comment #5)
> OK I took another look at this and I am -1 on making the change you suggest.
> 
> Firstly TilesUtilImpl would be the wrong place to do this, since that 
> implementation (as per its javadocs) is intended for use without Struts - the 
> correct place would be in either TilesUtilStrutsImpl or 
> TilesUtilStrutsModulesImpl.
> 
Sounds ok to me.

To help understand the need for a patch somehow let me explain though what it 
was needed for.
The fix actually resulted from a disfunctioning we faced when integrating Tiles 
in a project using already Struts.
The project used a personalization of the RequestProcessor which basically 
rewrote doForward() and doInclude() methods to fit some specific multi-inteface 
requirements.
Integrating Tiles as it is simply broke up that personalization.
View rendering just did not work any more.

> The main reason I'm against this though is that IMO it is architecturally 
wrong 
> and doing what you suggest would be against the contract of the 
> RequestProcessor. The RequestProcessor performs a whole set of steps to 
process 
> a request and just performing one of those steps would break its contract. If 
>  
> code was introduced into the RequestProcessors doInclude() method that relied 
> on something that was done in a prior step it could cause your change to 
break. 
> Also, tiles is only invloved in the "view rendering" portion of processing a 
> request - having tiles invoke the RequestProcessor is the wrong way round, 
> its 
> the RequestProcessor that should (and does in the TileRequestProcessor) 
delgate 
> part of its process to tiles. 

There is yet a missing step somewhere.

> TilesUtil as its name implies is just a utility class and the only place the 
> doInclude() method is called is in the InsertTag - and the functionality 
> provided (including the content of a specified uri) is what is required.

In that case the border between "view rendering" and actual request processing 
is just not as clear as that. 
The problem we had to provide a patch for actually underlines that fact (see 
above).
Maybe the solution you're pointed out is to implement the same mechanism in 
both 
places although that's functionality/code duplication and it does not convince 
me.
Especially because it actually fails in the view rendering independency/
transparency in respect with some other architectural components somehow.

So, in the case of a Tiles-Struts integration, I still believe that there is 
place for such a fix, certainly in different place.
But it remains necessary to process includes in a consitent way with how they 
get processed by the Request Processor (here I'm talking of Tile-Struts 
integration, not Tiles standalone).
So this may go in TilesUtilStrutsImpl.

Basically what I'm saying is that in the case of Tiles-Struts integration the 
"include" mechanism used for the view rendering cannot ignore the "include" 
mechanism used in the RequestProcessor.



-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2005-11-07 20:25 ---
OK I took another look at this and I am -1 on making the change you suggest.

Firstly TilesUtilImpl would be the wrong place to do this, since that 
implementation (as per its javadocs) is intended for use without Struts - the 
correct place would be in either TilesUtilStrutsImpl or 
TilesUtilStrutsModulesImpl.

The main reason I'm against this though is that IMO it is architecturally wrong 
and doing what you suggest would be against the contract of the 
RequestProcessor. The RequestProcessor performs a whole set of steps to process 
a request and just performing one of those steps would break its contract. If  
code was introduced into the RequestProcessors doInclude() method that relied 
on something that was done in a prior step it could cause your change to break. 
Also, tiles is only invloved in the "view rendering" portion of processing a 
request - having tiles invoke the RequestProcessor is the wrong way round, its 
the RequestProcessor that should (and does in the TileRequestProcessor) delgate 
part of its process to tiles. 

TilesUtil as its name implies is just a utility class and the only place the 
doInclude() method is called is in the InsertTag - and the functionality 
provided (including the content of a specified uri) is what is required.

Thirdly, I believe TilesUtil has been designed so that its behaviour can be 
customized and any Web App can implement its own TilesUtil behaviour as a 
subclass of TilesUtilImpl and therefore we are meeting the goals that you 
desire.

I think we should mark this as INVALID or WONTFIX.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2005-11-07 10:17 ---
(In reply to comment #3)
> In Struts 1.3.x the TilesRequestProcessor is replaced by the 
> TilesPreProcessor 
> Command. Further down the road there are plans for "standalone tiles" 
> (currently in the sandbox).
> 
> I'm guessing those developements probably make this request redundant? 

I agree this may be redundant with latest developments in 1.3.x branches.
But this is definitely not the case for struts versions up to 1.2.x.
So it still needs to be fixed there...


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703





--- Additional Comments From [EMAIL PROTECTED]  2005-11-06 04:56 ---
In Struts 1.3.x the TilesRequestProcessor is replaced by the TilesPreProcessor 
Command. Further down the road there are plans for "standalone tiles" 
(currently in the sandbox).

I'm guessing those developements probably make this request redundant? 

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

DO NOT REPLY [Bug 35703] - [tiles] TilesUtilImpl doInclude() should call TilesRequestProcessor doInclude()... but it doesn't

[bugzilla]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35703


[EMAIL PROTECTED] changed:

   What|Removed |Added

Summary|TilesUtilImpl doInclude()   |[tiles] TilesUtilImpl
   |should call |doInclude() should call
   |TilesRequestProcessor   |TilesRequestProcessor
   |doInclude()... but it   |doInclude()... but it
   |doesn't |doesn't




-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]