You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jetspeed-dev@portals.apache.org by Raphaël Luta <ra...@apache.org> on 2005/04/10 17:30:14 UTC
PortletURLProviderImpl.toString() ?
I was looking at J2 source code trying to figure out where to
modify the source code to allow a portal parameter to be considered
as a portlet parameter for all portlets found on the same page when
I encountered the following code in
org.apache.jetspeed.services.information.PortletURLProviderImpl
public String toString()
{
if ( clearParameters )
{
if (parameters != null)
{
Iterator names = parameters.keySet().iterator();
Map map = new HashMap();
while (names.hasNext())
{
String name = (String) names.next();
Object value = parameters.get(name);
String[] values = value instanceof String ? new
String[] {(String) value } : (String[]) value;
map.put(name,values);
}
}
return
url.createPortletURL(portletWindow,parameters,mode,state,action,secure);
}
else
{
return url.createPortletURL(portletWindow,mode,state,secure);
}
}
This seems clearly wrong to me since we create a new local HashMap (map)
that is never atually used. If someone can clue me in what this code is
supposed to do, I'll fix it.
--
Raphaël Luta - raphael@apache.org
Apache Portals - Enterprise Portal in Java
http://portals.apache.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org
Re: PortletURLProviderImpl.toString() ?
Posted by Ate Douma <at...@douma.nu>.
Raphaël and others,
Sorry for the delay.
My laptop crashed a week ago and only since this weekend do I have a new
developer environment to work with (my laptop still isn't fixed though
:-( ).
But, I've now fixed http://issues.apache.org/jira/browse/JS2-194 and
http://issues.apache.org/jira/browse/JS2-231.
Furthermore, I've cleaned up PortletURLProviderImpl.
So, if you have time, please test it out!
Regards, Ate
Ate Douma wrote:
>
>
> Raphaël Luta wrote:
>
>> Ate Douma wrote:
>>
>>> Raphaël Luta wrote:
>>> <snip>
>>>
>>> You're absolutely correct Raphaël, this is wrong.
>>> It is a long time ago I've been looking at this and I can't remember
>>> anymore
>>> what I was thinking at the time I wrote it but indeed, this isn't good.
>>> The whole logic behind the if (clearparameters) statement is wrong
>>> although it
>>> more or less seems to do the job (but probably only because Pluto
>>> always calls
>>> clearParameters() before it calls setParameters(Map)).
>>>
>>> There is more going on here though. I've seen some reports by users
>>> complaining
>>> that previously defined renderparameters aren't removed even if not
>>> defined on
>>> a new PortletURL. This area in the code might be part of that problem.
>>> Most likely, I was working on that functionality but simply forgot to
>>> complete it
>>> when I rewrote the PortletURL encoding.
>>> Solving this properly might require changes to the NavigationalState
>>> implementation as well!
>>> I wouldn't want to implement a (too) quick fix for that.
>>>
>>
>> We indeed saw some strange behavior with render parameters that don't
>> seem to be cleared properly.
>>
>>> Starting from Wednesday, I expect to have more time on my hands to
>>> look at this
>>> and will try to resolve it. if you can't wait that long and/or it is
>>> blocking you
>>> go ahead and fix this part. I'll then pick it up after that.
>>>
>>
>> If you know what should be the correct behavior, I'll gladly let you
>> take care of this ! Trying to get a picture of how this stuff works is
>> a real mess given the number of facades used both on Jetspeed and Pluto
>> side.
>
> Yes, I think I know what the correct behavior should be ;-)
> I'll give it highest priority later this week and created new issue JS-231
> (with priority Critical) for it.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org
>
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org
Re: PortletURLProviderImpl.toString() ?
Posted by Ate Douma <at...@douma.nu>.
Raphaël Luta wrote:
> Ate Douma wrote:
>
>> Raphaël Luta wrote:
>> <snip>
>>
>> You're absolutely correct Raphaël, this is wrong.
>> It is a long time ago I've been looking at this and I can't remember
>> anymore
>> what I was thinking at the time I wrote it but indeed, this isn't good.
>> The whole logic behind the if (clearparameters) statement is wrong
>> although it
>> more or less seems to do the job (but probably only because Pluto
>> always calls
>> clearParameters() before it calls setParameters(Map)).
>>
>> There is more going on here though. I've seen some reports by users
>> complaining
>> that previously defined renderparameters aren't removed even if not
>> defined on
>> a new PortletURL. This area in the code might be part of that problem.
>> Most likely, I was working on that functionality but simply forgot to
>> complete it
>> when I rewrote the PortletURL encoding.
>> Solving this properly might require changes to the NavigationalState
>> implementation as well!
>> I wouldn't want to implement a (too) quick fix for that.
>>
>
> We indeed saw some strange behavior with render parameters that don't
> seem to be cleared properly.
>
>> Starting from Wednesday, I expect to have more time on my hands to
>> look at this
>> and will try to resolve it. if you can't wait that long and/or it is
>> blocking you
>> go ahead and fix this part. I'll then pick it up after that.
>>
>
> If you know what should be the correct behavior, I'll gladly let you
> take care of this ! Trying to get a picture of how this stuff works is
> a real mess given the number of facades used both on Jetspeed and Pluto
> side.
Yes, I think I know what the correct behavior should be ;-)
I'll give it highest priority later this week and created new issue JS-231
(with priority Critical) for it.
---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org
Re: PortletURLProviderImpl.toString() ?
Posted by Raphaël Luta <ra...@apache.org>.
Ate Douma wrote:
> Raphaël Luta wrote:
> <snip>
>
> You're absolutely correct Raphaël, this is wrong.
> It is a long time ago I've been looking at this and I can't remember
> anymore
> what I was thinking at the time I wrote it but indeed, this isn't good.
> The whole logic behind the if (clearparameters) statement is wrong
> although it
> more or less seems to do the job (but probably only because Pluto always
> calls
> clearParameters() before it calls setParameters(Map)).
>
> There is more going on here though. I've seen some reports by users
> complaining
> that previously defined renderparameters aren't removed even if not
> defined on
> a new PortletURL. This area in the code might be part of that problem.
> Most likely, I was working on that functionality but simply forgot to
> complete it
> when I rewrote the PortletURL encoding.
> Solving this properly might require changes to the NavigationalState
> implementation as well!
> I wouldn't want to implement a (too) quick fix for that.
>
We indeed saw some strange behavior with render parameters that don't
seem to be cleared properly.
> Starting from Wednesday, I expect to have more time on my hands to look
> at this
> and will try to resolve it. if you can't wait that long and/or it is
> blocking you
> go ahead and fix this part. I'll then pick it up after that.
>
If you know what should be the correct behavior, I'll gladly let you
take care of this ! Trying to get a picture of how this stuff works is
a real mess given the number of facades used both on Jetspeed and Pluto
side.
--
Raphaël Luta - raphael@apache.org
Apache Portals - Enterprise Portal in Java
http://portals.apache.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org
Re: PortletURLProviderImpl.toString() ?
Posted by Ate Douma <at...@douma.nu>.
Raphaël Luta wrote:
> I was looking at J2 source code trying to figure out where to
> modify the source code to allow a portal parameter to be considered
> as a portlet parameter for all portlets found on the same page when
> I encountered the following code in
>
> org.apache.jetspeed.services.information.PortletURLProviderImpl
>
> public String toString()
> {
> if ( clearParameters )
> {
> if (parameters != null)
> {
> Iterator names = parameters.keySet().iterator();
> Map map = new HashMap();
> while (names.hasNext())
> {
> String name = (String) names.next();
> Object value = parameters.get(name);
> String[] values = value instanceof String ? new
> String[] {(String) value } : (String[]) value;
> map.put(name,values);
> }
> }
> return
> url.createPortletURL(portletWindow,parameters,mode,state,action,secure);
> }
> else
> {
> return url.createPortletURL(portletWindow,mode,state,secure);
> }
> }
>
> This seems clearly wrong to me since we create a new local HashMap (map)
> that is never atually used. If someone can clue me in what this code is
> supposed to do, I'll fix it.
>
You're absolutely correct Raphaël, this is wrong.
It is a long time ago I've been looking at this and I can't remember anymore
what I was thinking at the time I wrote it but indeed, this isn't good.
The whole logic behind the if (clearparameters) statement is wrong although it
more or less seems to do the job (but probably only because Pluto always calls
clearParameters() before it calls setParameters(Map)).
There is more going on here though. I've seen some reports by users complaining
that previously defined renderparameters aren't removed even if not defined on
a new PortletURL. This area in the code might be part of that problem.
Most likely, I was working on that functionality but simply forgot to complete it
when I rewrote the PortletURL encoding.
Solving this properly might require changes to the NavigationalState implementation as well!
I wouldn't want to implement a (too) quick fix for that.
Starting from Wednesday, I expect to have more time on my hands to look at this
and will try to resolve it. if you can't wait that long and/or it is blocking you
go ahead and fix this part. I'll then pick it up after that.
Regards, Ate
---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org