You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2013/02/22 15:08:28 UTC
Review request for UrlResourceReference
Hi,
At
https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patch you
may see a patch that fixes https://issues.apache.org/jira/browse/WICKET-4942
, https://issues.apache.org/jira/browse/WICKET-4907 and
https://issues.apache.org/jira/browse/WICKET-4903
The problem is that I don't feel very proud of the solution.
UrlResourceReference's (URR) purpose is to make it possible to use a
ResourceReference when all you have is a url (absolute or relative)
There solved problems are:
1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
This breaks the provided url in URR by relativizing it
2) Until now URR was handled by BasicResourceReferenceMapper which is
wrapped by ParentFolderPlaceholderProvider and leads to prefixes like "::"
in the generated url
And what I don't feel happy with is CalculatedUrl. This is a specialization
of Url which UrlRenderer does not try to render. Just returns it as it is.
Do you have better ideas how to tell UrlRenderer to not touch the Url when
it comes from UrlResourceReference ?
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>
Re: Review request for UrlResourceReference
Posted by Martin Grigorov <mg...@apache.org>.
I've just committed the suggested patch + the improvement suggested by
Andrea Del Bene to use an interface.
If you have better solution please create a new ticket with a patch.
On Tue, Feb 26, 2013 at 9:38 PM, Martin Grigorov <mg...@apache.org>wrote:
> Hi,
>
>
> On Tue, Feb 26, 2013 at 9:25 PM, Nuno Pedro Jacinto <nu...@cern.ch>wrote:
>
>> It wouldn't be better to add a method that can provide information
>> concerning what must be done instead of adding more class/interface
>> verification?
>> I'm sorry if I'm being a little annoying with this, but every time I look
>> inside the code I see instanceof everywhere and the last time that I check,
>> this is not the most efficient thing. It would probably be better if you
>> try to start thinking on a different approach before the framework starts
>> to become heavy.
>>
>
> Thanks for your feedback.
> By "most efficient" I guess you mean from user point of view. Because
> "instanceof" is one of the most efficient JVM instructions, i.e. it is very
> fast.
>
> Please attach a patch with your solution to any of the tickets.
>
>
>
>>
>> Cheers
>> -----Original Message-----
>> From: Martin Grigorov [mailto:mgrigorov@apache.org]
>> Sent: 25 February 2013 20:40
>> To: dev@wicket.apache.org
>> Subject: Re: Review request for UrlResourceReference
>>
>> On Mon, Feb 25, 2013 at 8:49 PM, Andrea Del Bene <an.delbene@gmail.com
>> >wrote:
>>
>> > I think that the three issue could be solved also introducing an
>> > interface for those resource references that want to directly render
>> their URL.
>> > Something like:
>> >
>> > public interface IResourceRefUrlGenerator {
>> > public Url getUrl();
>> > }
>> >
>> > Then, inside ReqyestCycle's method renderUrl we can check if resource
>> > reference implements this interface and if so, we can delegate Url
>> > generation to it.
>> >
>>
>> This looks better than my approach!
>> Thanks!
>>
>>
>> > I've attached my patch at WICKET-4907
>> >
>> >> Hi,
>> >>
>> >> At
>> >> https://issues.apache.org/**jira/secure/attachment/**
>> >> 12570471/WICKET-4907.patch<https://issues.apache.org/jira/secure/atta
>> >> chment/12570471/WICKET-4907.patch>you
>> >> may see a patch that fixes https://issues.apache.org/**
>> >> jira/browse/WICKET-4942<https://issues.apache.org/jira/browse/WICKET-
>> >> 4942> ,
>> >> https://issues.apache.org/**jira/browse/WICKET-4907<https://issues.ap
>> >> ache.org/jira/browse/WICKET-4907>and
>> >>
>> >> https://issues.apache.org/**jira/browse/WICKET-4903<https://issues.ap
>> >> ache.org/jira/browse/WICKET-4903>
>> >>
>> >> The problem is that I don't feel very proud of the solution.
>> >>
>> >> UrlResourceReference's (URR) purpose is to make it possible to use a
>> >> ResourceReference when all you have is a url (absolute or relative)
>> >>
>> >> There solved problems are:
>> >> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a
>> Url.
>> >> This breaks the provided url in URR by relativizing it
>> >>
>> >> 2) Until now URR was handled by BasicResourceReferenceMapper which is
>> >> wrapped by ParentFolderPlaceholderProvide**r and leads to prefixes
>> >> like "::"
>> >> in the generated url
>> >>
>> >>
>> >> And what I don't feel happy with is CalculatedUrl. This is a
>> >> specialization of Url which UrlRenderer does not try to render. Just
>> >> returns it as it is.
>> >>
>> >> Do you have better ideas how to tell UrlRenderer to not touch the Url
>> >> when it comes from UrlResourceReference ?
>> >>
>> >>
>> >
>>
>>
>> --
>> Martin Grigorov
>> jWeekend
>> Training, Consulting, Development
>> http://jWeekend.com <http://jweekend.com/>
>>
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>
RE: Review request for UrlResourceReference
Posted by Nuno Pedro Jacinto <nu...@cern.ch>.
No, I'm talking from the servers point of view. For the developers maybe, discarding the fact that everyone has its own style, it's better to have your current approach because you can identify the components without having to define types that are harder to maintain and require changes. I may be wrong, but as far as I know the processor only does basic operations. All the rest requires several clock cycles and just because we are not writing the code to do it doesn't mean that is not there.
Can you please do a test?
I'm not sure in this moment of the class that I use, but the interface was List and the class was something from the java standard library that doesn't implements List. Choose one that has some parent classes and interfaces. Just put it a cycle calling instanceof and you will see how fast it is.
I know that the performance has increase a lot since the first versions of java, but is still consider, as far as I know and maybe you can ask other developers about it, not to be that efficient. By this I don't mean that is so bad that it shouldn't be used. Just that maybe it's being overused.
Sorry, I'm not pretending to better or smarter. It just looks strange, considering that Wicket is a framework to be used not only by people with experience but also with less experience that will do, and I know by experience, a lot of inefficient code, to see the load of this operation to increase on the its foundation. Sometimes an is... method or an enumeration (not to entering the extreme of constants) will do the same task and it will be a simple equals operation for the processor.
-----Original Message-----
From: Martin Grigorov [mailto:mgrigorov@apache.org]
Sent: 26 February 2013 20:38
To: dev@wicket.apache.org
Subject: Re: Review request for UrlResourceReference
Hi,
On Tue, Feb 26, 2013 at 9:25 PM, Nuno Pedro Jacinto <nu...@cern.ch>wrote:
> It wouldn't be better to add a method that can provide information
> concerning what must be done instead of adding more class/interface
> verification?
> I'm sorry if I'm being a little annoying with this, but every time I
> look inside the code I see instanceof everywhere and the last time
> that I check, this is not the most efficient thing. It would probably
> be better if you try to start thinking on a different approach before
> the framework starts to become heavy.
>
Thanks for your feedback.
By "most efficient" I guess you mean from user point of view. Because "instanceof" is one of the most efficient JVM instructions, i.e. it is very fast.
Please attach a patch with your solution to any of the tickets.
>
> Cheers
> -----Original Message-----
> From: Martin Grigorov [mailto:mgrigorov@apache.org]
> Sent: 25 February 2013 20:40
> To: dev@wicket.apache.org
> Subject: Re: Review request for UrlResourceReference
>
> On Mon, Feb 25, 2013 at 8:49 PM, Andrea Del Bene <an.delbene@gmail.com
> >wrote:
>
> > I think that the three issue could be solved also introducing an
> > interface for those resource references that want to directly render
> their URL.
> > Something like:
> >
> > public interface IResourceRefUrlGenerator {
> > public Url getUrl();
> > }
> >
> > Then, inside ReqyestCycle's method renderUrl we can check if
> > resource reference implements this interface and if so, we can
> > delegate Url generation to it.
> >
>
> This looks better than my approach!
> Thanks!
>
>
> > I've attached my patch at WICKET-4907
> >
> >> Hi,
> >>
> >> At
> >> https://issues.apache.org/**jira/secure/attachment/**
> >> 12570471/WICKET-4907.patch<https://issues.apache.org/jira/secure/at
> >> ta chment/12570471/WICKET-4907.patch>you
> >> may see a patch that fixes https://issues.apache.org/**
> >> jira/browse/WICKET-4942<https://issues.apache.org/jira/browse/WICKE
> >> T-
> >> 4942> ,
> >> https://issues.apache.org/**jira/browse/WICKET-4907<https://issues.
> >> ap ache.org/jira/browse/WICKET-4907>and
> >>
> >> https://issues.apache.org/**jira/browse/WICKET-4903<https://issues.
> >> ap
> >> ache.org/jira/browse/WICKET-4903>
> >>
> >> The problem is that I don't feel very proud of the solution.
> >>
> >> UrlResourceReference's (URR) purpose is to make it possible to use
> >> a ResourceReference when all you have is a url (absolute or
> >> relative)
> >>
> >> There solved problems are:
> >> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves
> >> a
> Url.
> >> This breaks the provided url in URR by relativizing it
> >>
> >> 2) Until now URR was handled by BasicResourceReferenceMapper which
> >> is wrapped by ParentFolderPlaceholderProvide**r and leads to
> >> prefixes like "::"
> >> in the generated url
> >>
> >>
> >> And what I don't feel happy with is CalculatedUrl. This is a
> >> specialization of Url which UrlRenderer does not try to render.
> >> Just returns it as it is.
> >>
> >> Do you have better ideas how to tell UrlRenderer to not touch the
> >> Url when it comes from UrlResourceReference ?
> >>
> >>
> >
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>
Re: Review request for UrlResourceReference
Posted by Martin Grigorov <mg...@apache.org>.
Hi,
On Tue, Feb 26, 2013 at 9:25 PM, Nuno Pedro Jacinto <nu...@cern.ch>wrote:
> It wouldn't be better to add a method that can provide information
> concerning what must be done instead of adding more class/interface
> verification?
> I'm sorry if I'm being a little annoying with this, but every time I look
> inside the code I see instanceof everywhere and the last time that I check,
> this is not the most efficient thing. It would probably be better if you
> try to start thinking on a different approach before the framework starts
> to become heavy.
>
Thanks for your feedback.
By "most efficient" I guess you mean from user point of view. Because
"instanceof" is one of the most efficient JVM instructions, i.e. it is very
fast.
Please attach a patch with your solution to any of the tickets.
>
> Cheers
> -----Original Message-----
> From: Martin Grigorov [mailto:mgrigorov@apache.org]
> Sent: 25 February 2013 20:40
> To: dev@wicket.apache.org
> Subject: Re: Review request for UrlResourceReference
>
> On Mon, Feb 25, 2013 at 8:49 PM, Andrea Del Bene <an.delbene@gmail.com
> >wrote:
>
> > I think that the three issue could be solved also introducing an
> > interface for those resource references that want to directly render
> their URL.
> > Something like:
> >
> > public interface IResourceRefUrlGenerator {
> > public Url getUrl();
> > }
> >
> > Then, inside ReqyestCycle's method renderUrl we can check if resource
> > reference implements this interface and if so, we can delegate Url
> > generation to it.
> >
>
> This looks better than my approach!
> Thanks!
>
>
> > I've attached my patch at WICKET-4907
> >
> >> Hi,
> >>
> >> At
> >> https://issues.apache.org/**jira/secure/attachment/**
> >> 12570471/WICKET-4907.patch<https://issues.apache.org/jira/secure/atta
> >> chment/12570471/WICKET-4907.patch>you
> >> may see a patch that fixes https://issues.apache.org/**
> >> jira/browse/WICKET-4942<https://issues.apache.org/jira/browse/WICKET-
> >> 4942> ,
> >> https://issues.apache.org/**jira/browse/WICKET-4907<https://issues.ap
> >> ache.org/jira/browse/WICKET-4907>and
> >>
> >> https://issues.apache.org/**jira/browse/WICKET-4903<https://issues.ap
> >> ache.org/jira/browse/WICKET-4903>
> >>
> >> The problem is that I don't feel very proud of the solution.
> >>
> >> UrlResourceReference's (URR) purpose is to make it possible to use a
> >> ResourceReference when all you have is a url (absolute or relative)
> >>
> >> There solved problems are:
> >> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a
> Url.
> >> This breaks the provided url in URR by relativizing it
> >>
> >> 2) Until now URR was handled by BasicResourceReferenceMapper which is
> >> wrapped by ParentFolderPlaceholderProvide**r and leads to prefixes
> >> like "::"
> >> in the generated url
> >>
> >>
> >> And what I don't feel happy with is CalculatedUrl. This is a
> >> specialization of Url which UrlRenderer does not try to render. Just
> >> returns it as it is.
> >>
> >> Do you have better ideas how to tell UrlRenderer to not touch the Url
> >> when it comes from UrlResourceReference ?
> >>
> >>
> >
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>
RE: Review request for UrlResourceReference
Posted by Nuno Pedro Jacinto <nu...@cern.ch>.
It wouldn't be better to add a method that can provide information concerning what must be done instead of adding more class/interface verification?
I'm sorry if I'm being a little annoying with this, but every time I look inside the code I see instanceof everywhere and the last time that I check, this is not the most efficient thing. It would probably be better if you try to start thinking on a different approach before the framework starts to become heavy.
Cheers
-----Original Message-----
From: Martin Grigorov [mailto:mgrigorov@apache.org]
Sent: 25 February 2013 20:40
To: dev@wicket.apache.org
Subject: Re: Review request for UrlResourceReference
On Mon, Feb 25, 2013 at 8:49 PM, Andrea Del Bene <an...@gmail.com>wrote:
> I think that the three issue could be solved also introducing an
> interface for those resource references that want to directly render their URL.
> Something like:
>
> public interface IResourceRefUrlGenerator {
> public Url getUrl();
> }
>
> Then, inside ReqyestCycle's method renderUrl we can check if resource
> reference implements this interface and if so, we can delegate Url
> generation to it.
>
This looks better than my approach!
Thanks!
> I've attached my patch at WICKET-4907
>
>> Hi,
>>
>> At
>> https://issues.apache.org/**jira/secure/attachment/**
>> 12570471/WICKET-4907.patch<https://issues.apache.org/jira/secure/atta
>> chment/12570471/WICKET-4907.patch>you
>> may see a patch that fixes https://issues.apache.org/**
>> jira/browse/WICKET-4942<https://issues.apache.org/jira/browse/WICKET-
>> 4942> ,
>> https://issues.apache.org/**jira/browse/WICKET-4907<https://issues.ap
>> ache.org/jira/browse/WICKET-4907>and
>>
>> https://issues.apache.org/**jira/browse/WICKET-4903<https://issues.ap
>> ache.org/jira/browse/WICKET-4903>
>>
>> The problem is that I don't feel very proud of the solution.
>>
>> UrlResourceReference's (URR) purpose is to make it possible to use a
>> ResourceReference when all you have is a url (absolute or relative)
>>
>> There solved problems are:
>> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
>> This breaks the provided url in URR by relativizing it
>>
>> 2) Until now URR was handled by BasicResourceReferenceMapper which is
>> wrapped by ParentFolderPlaceholderProvide**r and leads to prefixes
>> like "::"
>> in the generated url
>>
>>
>> And what I don't feel happy with is CalculatedUrl. This is a
>> specialization of Url which UrlRenderer does not try to render. Just
>> returns it as it is.
>>
>> Do you have better ideas how to tell UrlRenderer to not touch the Url
>> when it comes from UrlResourceReference ?
>>
>>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>
Re: Review request for UrlResourceReference
Posted by Martin Grigorov <mg...@apache.org>.
On Mon, Feb 25, 2013 at 8:49 PM, Andrea Del Bene <an...@gmail.com>wrote:
> I think that the three issue could be solved also introducing an interface
> for those resource references that want to directly render their URL.
> Something like:
>
> public interface IResourceRefUrlGenerator
> {
> public Url getUrl();
> }
>
> Then, inside ReqyestCycle's method renderUrl we can check if resource
> reference implements this interface and if so, we can delegate Url
> generation to it.
>
This looks better than my approach!
Thanks!
> I've attached my patch at WICKET-4907
>
>> Hi,
>>
>> At
>> https://issues.apache.org/**jira/secure/attachment/**
>> 12570471/WICKET-4907.patch<https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patch>you
>> may see a patch that fixes https://issues.apache.org/**
>> jira/browse/WICKET-4942<https://issues.apache.org/jira/browse/WICKET-4942>
>> , https://issues.apache.org/**jira/browse/WICKET-4907<https://issues.apache.org/jira/browse/WICKET-4907>and
>>
>> https://issues.apache.org/**jira/browse/WICKET-4903<https://issues.apache.org/jira/browse/WICKET-4903>
>>
>> The problem is that I don't feel very proud of the solution.
>>
>> UrlResourceReference's (URR) purpose is to make it possible to use a
>> ResourceReference when all you have is a url (absolute or relative)
>>
>> There solved problems are:
>> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
>> This breaks the provided url in URR by relativizing it
>>
>> 2) Until now URR was handled by BasicResourceReferenceMapper which is
>> wrapped by ParentFolderPlaceholderProvide**r and leads to prefixes like
>> "::"
>> in the generated url
>>
>>
>> And what I don't feel happy with is CalculatedUrl. This is a
>> specialization
>> of Url which UrlRenderer does not try to render. Just returns it as it is.
>>
>> Do you have better ideas how to tell UrlRenderer to not touch the Url when
>> it comes from UrlResourceReference ?
>>
>>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>
Re: Review request for UrlResourceReference
Posted by Andrea Del Bene <an...@gmail.com>.
I think that the three issue could be solved also introducing an
interface for those resource references that want to directly render
their URL. Something like:
public interface IResourceRefUrlGenerator
{
public Url getUrl();
}
Then, inside ReqyestCycle's method renderUrl we can check if resource
reference implements this interface and if so, we can delegate Url
generation to it.
I've attached my patch at WICKET-4907
> Hi,
>
> At
> https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patch you
> may see a patch that fixes https://issues.apache.org/jira/browse/WICKET-4942
> , https://issues.apache.org/jira/browse/WICKET-4907 and
> https://issues.apache.org/jira/browse/WICKET-4903
>
> The problem is that I don't feel very proud of the solution.
>
> UrlResourceReference's (URR) purpose is to make it possible to use a
> ResourceReference when all you have is a url (absolute or relative)
>
> There solved problems are:
> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
> This breaks the provided url in URR by relativizing it
>
> 2) Until now URR was handled by BasicResourceReferenceMapper which is
> wrapped by ParentFolderPlaceholderProvider and leads to prefixes like "::"
> in the generated url
>
>
> And what I don't feel happy with is CalculatedUrl. This is a specialization
> of Url which UrlRenderer does not try to render. Just returns it as it is.
>
> Do you have better ideas how to tell UrlRenderer to not touch the Url when
> it comes from UrlResourceReference ?
>
Re: Review request for UrlResourceReference
Posted by Sven Meier <sv...@meiers.net>.
I'll take a look at it this weekend.
Thanks for your patch
Sven
On 02/22/2013 03:08 PM, Martin Grigorov wrote:
> Hi,
>
> At
> https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patch you
> may see a patch that fixes https://issues.apache.org/jira/browse/WICKET-4942
> , https://issues.apache.org/jira/browse/WICKET-4907 and
> https://issues.apache.org/jira/browse/WICKET-4903
>
> The problem is that I don't feel very proud of the solution.
>
> UrlResourceReference's (URR) purpose is to make it possible to use a
> ResourceReference when all you have is a url (absolute or relative)
>
> There solved problems are:
> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
> This breaks the provided url in URR by relativizing it
>
> 2) Until now URR was handled by BasicResourceReferenceMapper which is
> wrapped by ParentFolderPlaceholderProvider and leads to prefixes like "::"
> in the generated url
>
>
> And what I don't feel happy with is CalculatedUrl. This is a specialization
> of Url which UrlRenderer does not try to render. Just returns it as it is.
>
> Do you have better ideas how to tell UrlRenderer to not touch the Url when
> it comes from UrlResourceReference ?
>
Re: Review request for UrlResourceReference
Posted by Martin Grigorov <mg...@apache.org>.
On Mon, Feb 25, 2013 at 6:06 PM, Sven Meier <sv...@meiers.net> wrote:
> Maybe it's time to clean up Wicket's Url class. From its javadoc:
>
> >>> Represents the URL part <b>after Wicket Filter</b>
>
> It seems to me we're using a Url on many places where it wasn't intended
> to.
>
> How about a flag in url whether its wicket specific, context relative or
> global (i.e. with host).
I also considered this option.
But I find both options not perfect and used the one with specific type
because for me it is more clean.
>
>
> Sven
>
>
>
> On 02/22/2013 03:40 PM, Ernesto Reinaldo Barreiro wrote:
>
>> Martin,
>>
>> If what you want to avoid if the "if". Maybe UrlRenderer can
>> be split-ed into two sub-renderers. E.g. Have interface
>>
>> IUrlRenderer {
>>
>> boolean canRenderUrl(); // decides if it can renser URL.
>>
>> String renderUrl(URL url);
>> }
>>
>> and UrlRenderer contains a list of two IUrlRenderers... DefaultUrlRenderer
>> (previous behavior), and a new CalculatedUrlRenderer (before
>> DefaultUrlRenderer). This way it might be easier to plug new "special
>> cases" in the future. But that might be over-killing?
>>
>>
>> On Fri, Feb 22, 2013 at 3:08 PM, Martin Grigorov <mgrigorov@apache.org
>> >wrote:
>>
>> Hi,
>>>
>>> At
>>> https://issues.apache.org/**jira/secure/attachment/**
>>> 12570471/WICKET-4907.patchyou<https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patchyou>
>>> may see a patch that fixes
>>> https://issues.apache.org/**jira/browse/WICKET-4942<https://issues.apache.org/jira/browse/WICKET-4942>
>>> , https://issues.apache.org/**jira/browse/WICKET-4907<https://issues.apache.org/jira/browse/WICKET-4907>and
>>> https://issues.apache.org/**jira/browse/WICKET-4903<https://issues.apache.org/jira/browse/WICKET-4903>
>>>
>>> The problem is that I don't feel very proud of the solution.
>>>
>>> UrlResourceReference's (URR) purpose is to make it possible to use a
>>> ResourceReference when all you have is a url (absolute or relative)
>>>
>>> There solved problems are:
>>> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
>>> This breaks the provided url in URR by relativizing it
>>>
>>> 2) Until now URR was handled by BasicResourceReferenceMapper which is
>>> wrapped by ParentFolderPlaceholderProvide**r and leads to prefixes like
>>> "::"
>>> in the generated url
>>>
>>>
>>> And what I don't feel happy with is CalculatedUrl. This is a
>>> specialization
>>> of Url which UrlRenderer does not try to render. Just returns it as it
>>> is.
>>>
>>> Do you have better ideas how to tell UrlRenderer to not touch the Url
>>> when
>>> it comes from UrlResourceReference ?
>>>
>>> --
>>> Martin Grigorov
>>> jWeekend
>>> Training, Consulting, Development
>>> http://jWeekend.com <http://jweekend.com/>
>>>
>>>
>>
>>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>
Re: Review request for UrlResourceReference
Posted by Sven Meier <sv...@meiers.net>.
Maybe it's time to clean up Wicket's Url class. From its javadoc:
>>> Represents the URL part <b>after Wicket Filter</b>
It seems to me we're using a Url on many places where it wasn't intended to.
How about a flag in url whether its wicket specific, context relative or
global (i.e. with host).
Sven
On 02/22/2013 03:40 PM, Ernesto Reinaldo Barreiro wrote:
> Martin,
>
> If what you want to avoid if the "if". Maybe UrlRenderer can
> be split-ed into two sub-renderers. E.g. Have interface
>
> IUrlRenderer {
>
> boolean canRenderUrl(); // decides if it can renser URL.
>
> String renderUrl(URL url);
> }
>
> and UrlRenderer contains a list of two IUrlRenderers... DefaultUrlRenderer
> (previous behavior), and a new CalculatedUrlRenderer (before
> DefaultUrlRenderer). This way it might be easier to plug new "special
> cases" in the future. But that might be over-killing?
>
>
> On Fri, Feb 22, 2013 at 3:08 PM, Martin Grigorov <mg...@apache.org>wrote:
>
>> Hi,
>>
>> At
>> https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patchyou
>> may see a patch that fixes
>> https://issues.apache.org/jira/browse/WICKET-4942
>> , https://issues.apache.org/jira/browse/WICKET-4907 and
>> https://issues.apache.org/jira/browse/WICKET-4903
>>
>> The problem is that I don't feel very proud of the solution.
>>
>> UrlResourceReference's (URR) purpose is to make it possible to use a
>> ResourceReference when all you have is a url (absolute or relative)
>>
>> There solved problems are:
>> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
>> This breaks the provided url in URR by relativizing it
>>
>> 2) Until now URR was handled by BasicResourceReferenceMapper which is
>> wrapped by ParentFolderPlaceholderProvider and leads to prefixes like "::"
>> in the generated url
>>
>>
>> And what I don't feel happy with is CalculatedUrl. This is a specialization
>> of Url which UrlRenderer does not try to render. Just returns it as it is.
>>
>> Do you have better ideas how to tell UrlRenderer to not touch the Url when
>> it comes from UrlResourceReference ?
>>
>> --
>> Martin Grigorov
>> jWeekend
>> Training, Consulting, Development
>> http://jWeekend.com <http://jweekend.com/>
>>
>
>
Re: Review request for UrlResourceReference
Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
Martin,
If what you want to avoid if the "if". Maybe UrlRenderer can
be split-ed into two sub-renderers. E.g. Have interface
IUrlRenderer {
boolean canRenderUrl(); // decides if it can renser URL.
String renderUrl(URL url);
}
and UrlRenderer contains a list of two IUrlRenderers... DefaultUrlRenderer
(previous behavior), and a new CalculatedUrlRenderer (before
DefaultUrlRenderer). This way it might be easier to plug new "special
cases" in the future. But that might be over-killing?
On Fri, Feb 22, 2013 at 3:08 PM, Martin Grigorov <mg...@apache.org>wrote:
> Hi,
>
> At
> https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patchyou
> may see a patch that fixes
> https://issues.apache.org/jira/browse/WICKET-4942
> , https://issues.apache.org/jira/browse/WICKET-4907 and
> https://issues.apache.org/jira/browse/WICKET-4903
>
> The problem is that I don't feel very proud of the solution.
>
> UrlResourceReference's (URR) purpose is to make it possible to use a
> ResourceReference when all you have is a url (absolute or relative)
>
> There solved problems are:
> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
> This breaks the provided url in URR by relativizing it
>
> 2) Until now URR was handled by BasicResourceReferenceMapper which is
> wrapped by ParentFolderPlaceholderProvider and leads to prefixes like "::"
> in the generated url
>
>
> And what I don't feel happy with is CalculatedUrl. This is a specialization
> of Url which UrlRenderer does not try to render. Just returns it as it is.
>
> Do you have better ideas how to tell UrlRenderer to not touch the Url when
> it comes from UrlResourceReference ?
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>
--
Regards - Ernesto Reinaldo Barreiro
Antilia Soft
http://antiliasoft.com/ <http://antiliasoft.com/antilia>
Re: Review request for UrlResourceReference
Posted by Andrea Del Bene <an...@gmail.com>.
Sorry, I've just noted that you use a complete new mapper for Url
resources....
> Hi Martin,
>
> I've looked at your patch but I still miss something. As reported in
> WICKET-4907, class ParentPathReferenceRewriter is the entity that
> physically substitutes ".." with the corresponding placeholder (by
> default "::"). But in your patch you didn't modify this class. Why?
>
>> Hi,
>>
>> At
>> https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patch
>> you
>> may see a patch that fixes
>> https://issues.apache.org/jira/browse/WICKET-4942
>> , https://issues.apache.org/jira/browse/WICKET-4907 and
>> https://issues.apache.org/jira/browse/WICKET-4903
>>
>> The problem is that I don't feel very proud of the solution.
>>
>> UrlResourceReference's (URR) purpose is to make it possible to use a
>> ResourceReference when all you have is a url (absolute or relative)
>>
>> There solved problems are:
>> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a
>> Url.
>> This breaks the provided url in URR by relativizing it
>>
>> 2) Until now URR was handled by BasicResourceReferenceMapper which is
>> wrapped by ParentFolderPlaceholderProvider and leads to prefixes like
>> "::"
>> in the generated url
>>
>>
>> And what I don't feel happy with is CalculatedUrl. This is a
>> specialization
>> of Url which UrlRenderer does not try to render. Just returns it as
>> it is.
>>
>> Do you have better ideas how to tell UrlRenderer to not touch the Url
>> when
>> it comes from UrlResourceReference ?
>>
>
Re: Review request for UrlResourceReference
Posted by Andrea Del Bene <an...@gmail.com>.
Hi Martin,
I've looked at your patch but I still miss something. As reported in
WICKET-4907, class ParentPathReferenceRewriter is the entity that
physically substitutes ".." with the corresponding placeholder (by
default "::"). But in your patch you didn't modify this class. Why?
> Hi,
>
> At
> https://issues.apache.org/jira/secure/attachment/12570471/WICKET-4907.patch you
> may see a patch that fixes https://issues.apache.org/jira/browse/WICKET-4942
> , https://issues.apache.org/jira/browse/WICKET-4907 and
> https://issues.apache.org/jira/browse/WICKET-4903
>
> The problem is that I don't feel very proud of the solution.
>
> UrlResourceReference's (URR) purpose is to make it possible to use a
> ResourceReference when all you have is a url (absolute or relative)
>
> There solved problems are:
> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a Url.
> This breaks the provided url in URR by relativizing it
>
> 2) Until now URR was handled by BasicResourceReferenceMapper which is
> wrapped by ParentFolderPlaceholderProvider and leads to prefixes like "::"
> in the generated url
>
>
> And what I don't feel happy with is CalculatedUrl. This is a specialization
> of Url which UrlRenderer does not try to render. Just returns it as it is.
>
> Do you have better ideas how to tell UrlRenderer to not touch the Url when
> it comes from UrlResourceReference ?
>