The whole point is that I still think we *don't* have to make
something like this as easy as possible. It is a one-per-application
thing, and just a piece of documentation of how to do this would
suffice. Even if that means writing a couple of lines instead of one.
What sucks with this current method is that it depends on a default of
a default. Hardly something that is guaranteed. Furthermore, looking
at how this was implementated in 1.1; I think that wasn't really good
either. A much better place would have been in RequestCycle instead of
Application. The only reason why it was/ is in application is that it
saved people from writing an extra line of code, not because the
(global) scoping made sense in the first place.
So... still... we should think about *not* putting this method in
Application, but instead tell people how to work with the
IRequestCycleProcessor instead. Overriding IRequestCycleProcessor is
not nescesarily the easiest thing, but in this case I really think it
doesn't matter much. You can proxy, reimplement, whatever, and the
scoping makes sense and is all in one place instead of scattered along
RequestCycle, Application and gee knows whereever we put our bits and
pieces in the past.
One last word about IExceptionResponseStrategy: that's an
implementation specific interface that is used by
AbstractCompoundRequestCycleProcessor and its inheriting classes. That
interface must *never* be part of a factory method in application
because who says you want to use AbstractCompoundRequestCycleProcessor
anyway.
Again, let me stress that I think it is important to get our scoping
etc better to avoid the kind of spaghetti we started to get in 1.1. It
is much better to write a couple of lines extra, but have a clear
contract and everything at the right place, then save a few lines of
code but having a mess. This is certainly the case for code you write
once a project.
Eelco
Eelco
On 12/19/05, Johan Compagner <[EMAIL PROTECTED]> wrote:
> That looks fine to me!
>
> How would you do it better? Just let them make there own
> DefaultExceptionResponseProcessor ?
> And implement it?
> But because of that they have to override protected IRequestCycleProcessor
> getDefaultRequestCycleProcessor() of application?
> And do what you do below ?
> What happens then if we refactor are improve the default behaviour of the
> getDefaultREquestCycleProcessor()?
>
> I as a developer don't want to implement that method if i only want to
> override one of the things like WebRequestEncoder
> Because then i lose suddenly the other behaviour that wicket can change or
> improve!
>
> What i think would be handy was this:
>
> {
> if (requestCycleProcessor == null)
> {
> requestCycleProcessor = new
> CompoundRequestCycleProcessor(getWebRequestEncoder(),
> getDefaultEventProcessorStrategy(),
> getDefaultExceptionResponseProcessor());
> > }
> > return requestCycleProcessor;
> >
>
> > }
> >
>
>
> And better yet for all 5 of them so also the IEventProcessorStrategy
> eventProcessorStrategy and so on.
> Then people can just reimplement that get method and return just the thing
> they want to change.
> Then in i don't care that the onRuntimeException is removed from application
> to just the ExceptionProcessor
>
> because i can just do:
>
> protected IExceptionResponseStrategy
> getDefaultExceptionResponseProcessor()
> {
> return new DefaultExceptionResponseProcessor()
> {
> > protected Page onRuntimeException(Page page, RuntimeException e)
> > {
> > // handle my exception.
> > }
>
> > });
>
> I just don't want to suddenly know about all the other stuff like
> WebEncoder/CompoundProcessor just when i want to override the exception page
> behaviour.
>
> johan
>
>
>
> On 12/19/05, Eelco Hillenius <[EMAIL PROTECTED]> wrote:
> >
> > Well, you don't have to do that. You only have to override
> > getDefaultRequestCycleProcessor and provide your own
> > ExceptionResponseProcessor for it.
> >
> > Anyway, I put the method back in using the default request cycle
> > processor. I still don't agree, but I seem to have lost to yet another
> > corner case. Anyway, here it is:
> >
> > /**
> > * Gets the default request cycle processor (with lazy
> initialization). This
> > * is the [EMAIL PROTECTED] IRequestCycleProcessor} that will be
> > used by
> > * [EMAIL PROTECTED] RequestCycle}s when custom implementations of
> > the
> request cycle
> > * do not provide their own customized versions.
> > *
> > * @return the default request cycle processor
> > */
> > protected IRequestCycleProcessor
> getDefaultRequestCycleProcessor()
> > {
> > if (requestCycleProcessor == null)
> > {
> > requestCycleProcessor = new
> CompoundRequestCycleProcessor(new
> > WebRequestEncoder(),
> > new
> DefaultEventProcessorStrategy(), new
> > DefaultExceptionResponseProcessor()
> > {
> > protected
> Page onRuntimeException(Page page, RuntimeException e)
> > {
> >
> return
> WebApplication.this.onRuntimeException(page, e);
> > }
> > });
> > }
> > return requestCycleProcessor;
> > }
> >
> > /**
> > * Template method that is called when a runtime exception is
> thrown, just
> > * before the actual handling of the runtime exception.
> > *
> > * @param page
> > * Any page context where the exception was thrown
> > * @param e
> > * The exception
> > * @return Any error page to redirect to
> > */
> > protected Page onRuntimeException(Page page, RuntimeException e)
> > {
> > return null;
> > }
> >
> > It's in CVS now.
> >
> > Eelco
> >
> >
> >
> > On 12/18/05, Igor Vaynberg <[EMAIL PROTECTED]> wrote:
> > > i agree with Johan. I think it is something every application will need,
> > > even for as much as logging, and it sucks to create a request cycle
> factory
> > > with your own request cycle, etc. i dont think there is anything wrong
> with
> > > a convinience method that is used often. make easy things easier and
> hard
> > > things possible.
> > >
> > > -Igor
> > >
> > >
> > >
> > > On 12/18/05, Johan Compagner < [EMAIL PROTECTED]> wrote:
> > > > removing that method will break a lot of apps i think
> > > > Many people are using it.
> > > >
> > > > handling an exception is for me an application issue. So to say that i
> > > find that in the wrong place.. don't think so.
> > > >
> > > >
> > > > johan
> > > >
> > > >
> > > >
> > > > On 12/18/05, Eelco Hillenius < [EMAIL PROTECTED] > wrote:
> > > > > I'll work on a new proposal now.
> > > > >
> > > > > Eelco
> > > > >
> > > > >
> > > > > On 12/18/05, Eelco Hillenius < [EMAIL PROTECTED] > wrote:
> > > > > > But why 'just handling exceptions'. It is quite a non-standard
> thing
> > > > > > you are trying to do. Furthermore, there is
> > > > > > WebApplication.getDefaultRequestCycleFactory so
> it is
> > > not that much
> > > > > > work.
> > > > > >
> > > > > > But like I said, I am not in favor of having such a method in the
> > > first place as
> > > > > >
> > > > > > protected Page onRuntimeException(final Page page, final
> > > RuntimeException e)
> > > > > >
> > > > > > gives you crappy info. There doesn't even need to be a page; it
> should
> > > > > > be request target in the current state, which could mean anything
> from
> > > > > > a page to a shared resource to an external resource. Furthermore,
> the
> > > > > > only thing you should allow this method to do is to use the
> request
> > > > > > processing strategy. If we don't do this, we can't even guarantee
> the
> > > > > > contract of that strategy. And finally, the application scope just
> > > > > > sucks. We can put everything in the semi-global application scope,
> > > > > > can't we? That's allways the easiest solution, but is not very
> object
> > > > > > oriented. Finally, we shouldn't be too uptight about having a few
> > > > > > lines more if that makes sense for something you do once in your
> > > > > > project/ application if people use it at all.
> > > > > >
> > > > > > Eelco
> > > > > >
> > > > > >
> > > > > > On 12/18/05, Johan Compagner < [EMAIL PROTECTED]> wrote:
> > > > > > > Don't move it to the request cycle !
> > > > > > > There it was really at the beginning. But that is just horrible
> > > because what
> > > > > > > is needed to be done before you can override an request
> cycle....
> > > > > > >
> > > > > > >
> > >
> Application.getSessionFactory().newSession().getRequestCycleFactory().newRequestCycle()
> > > > > > >
> > > > > > > So you have to make 4 classes to have youre own request cycle.
> That
> > > is
> > > > > > > horrible for just handling exceptions.
> > > > > > >
> > > > > > > johan
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On 12/18/05, Eelco Hillenius < [EMAIL PROTECTED] >
> wrote:
> > > > > > > >
> > > > > > > > I'm not to happy to have a method like that in the Application
> > > object
> > > > > > > > in the first place. First of all, as you can replace the
> request
> > > > > > > > processing strategy, it is in fact not really needed, but mere
> > > > > > > > convenience. And as a convenience method, I think it would
> make
> > > much
> > > > > > > > more sense to have that (still overridable) method part of the
> > > request
> > > > > > > > cycle. Imo, the scoping would be more logical. But really, I'm
> > > still
> > > > > > > > thinking about whether we should provide such a convenience
> method
> > > in
> > > > > > > > the first place.
> > > > > > > >
> > > > > > > > Eelco
> > > > > > > >
> > > > > > > >
> > > > > > > > On 12/18/05, Igor Vaynberg < [EMAIL PROTECTED]> wrote:
> > > > > > > > > hi guys, looks like our latest request cycle refactoring
> broke
> > > > > > > > > onRuntimeException handler in Application. i looked into
> fixing
> > > it and
> > > > > > > there
> > > > > > > > > doesnt seem to be a very clean way.
> > > > > > > > >
> > > > > > > > > first, we no longer deal with a runtime exception, our
> > > > > > > > > IExceptionResponseStrategy now deals with a regular
> exception.
> > > so i
> > > > > > > renamed
> > > > > > > > > onRuntimeException to onException and it now takes a regular
> > > exception
> > > > > > > as a
> > > > > > > > > param.
> > > > > > > > >
> > > > > > > > > secondly it is now public since the response strategy and
> > > application
> > > > > > > are
> > > > > > > > > no longer in the same package.
> > > > > > > > >
> > > > > > > > > finally there is also some weirdness in mock application
> since
> > > you cant
> > > > > > > > > rethrow a checked exception without declaring it so i have
> to
> > > wrap it in
> > > > > > > a
> > > > > > > > > runtime exception which is not what user might expect.
> > > > > > > > >
> > > > > > > > > im attaching the patch. if no one is against this fix i
> will
> > > commit it.
> > > > > > > > > just wanted to check since this will break some clients.
> > > > > > > > >
> > > > > > > > > -Igor
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > -------------------------------------------------------
> > > > > > > > This SF.net email is sponsored by: Splunk Inc. Do you grep
> through
> > > log
> > > > > > > files
> > > > > > > > for problems? Stop! Download the new AJAX search engine that
> > > makes
> > > > > > > > searching your log files as easy as surfing the web.
> DOWNLOAD
> > > SPLUNK!
> > > > > > > >
> > > http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > > > > > > >
> _______________________________________________
> > > > > > > > Wicket-develop mailing list
> > > > > > > > [email protected]
> > > > > > > >
> > > > > > >
> > >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> -------------------------------------------------------
> > > > > This SF.net email is sponsored by: Splunk Inc. Do you grep through
> log
> > > files
> > > > > for problems? Stop! Download the new AJAX search engine that makes
> > > > > searching your log files as easy as surfing the web. DOWNLOAD
> SPLUNK!
> > > > > http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > > > > _______________________________________________
> > > > > Wicket-develop mailing list
> > > > > [email protected]
> > > > >
> > >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> > > > >
> > > >
> > > >
> > >
> > >
> >
> >
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files
> > for problems? Stop! Download the new AJAX search engine that makes
> > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
> > http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > _______________________________________________
> > Wicket-develop mailing list
> > [email protected]
> >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >
>
>
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_idv37&alloc_id865&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
