You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pluto-dev@portals.apache.org by Eric Dalquist <ed...@unicon.net> on 2004/07/07 20:03:04 UTC
Working on fix for bug PLUTO-55
I'm working on a bug fix for http://nagoya.apache.org/jira/browse/PLUTO-55
I've replaced the simple constant check for a "text/html" content type
with the following (RenderResponseImpl.java):
private boolean isValidContentType(String type)
{
PortletWindow window = this.getInternalPortletWindow();
PortletEntity entity = window.getPortletEntity();
PortletDefinition definition = entity.getPortletDefinition();
ContentTypeSet contentTypes = definition.getContentTypeSet();
//TODO: there appears to be code to see if a specific mode
supports the
// content type ... I need to figure out how to know what
PortletMode
// we are in at this point in the code.
for (Iterator ctItr = contentTypes.iterator(); ctItr.hasNext(); ) {
ContentType ct = (ContentType)ctItr.next();
if (ct.getContentType().equalsIgnoreCase(type)) {
return true;
}
}
return false;
}
This works and is better than what was there before but there is still a
part missing. The ContentType should actually be checked against a
PortletMode as well. ContentType has a supportsPortletMode |method which
is passed a PortletMode and returns true or false if that ContentType is
supported for the specified mode.
I'm not sure how to get the current PortletMode in the RenderResponseImpl.
Any suggestions would be much appreciated.
-Eric Dalquist
|
Re: Working on fix for bug PLUTO-55
Posted by Eric Dalquist <ed...@unicon.net>.
Thank you very much!
-Eric Dalquist
David Sean Taylor wrote:
>
> On Jul 8, 2004, at 4:13 PM, Eric Dalquist wrote:
>
>> No, my first patch was a bit hasty, I didn't quite understand what
>> the spec said when I wrote it. The workaround patch would be to take
>> that method and simply return false in the body. I'll attach a CVS
>> diff with this email. If you could incorporate this into CVS it
>> would be very much appreciated.
>>
> patch applied
>
>> Thank you,
>> Eric Dalquist
>>
>> David Sean Taylor wrote:
>>
>>>
>>> On Jul 8, 2004, at 12:38 PM, Eric Dalquist wrote:
>>>
>>>> Dave,
>>>> Thank you for the reply and I would be happy to but I cannot
>>>> figure out how to create the patch without making changes to quite
>>>> a few method signatures so the RenderResponse has a reference to
>>>> the RenderRequest. Without that reference I'm not sure how the
>>>> RenderResponse is supposed to know what the valid content types
>>>> returned by the getResponseContentType method on RenderRequest
>>>> are. I can provide the code for the suggested interim fix which
>>>> simply allows any content type to be set on the RenderResponse.
>>>>
>>> Is that the patch provided in your previous email?
>>>
>> Index: RenderResponseImpl.java
>> ===================================================================
>> RCS file:
>> /home/cvspublic/jakarta-pluto/container/src/java/org/apache/pluto/
>> core/impl/RenderResponseImpl.java,v
>> retrieving revision 1.6
>> diff -u -r1.6 RenderResponseImpl.java
>> --- RenderResponseImpl.java 19 Mar 2004 12:40:21 -0000 1.6
>> +++ RenderResponseImpl.java 8 Jul 2004 23:11:42 -0000
>> @@ -154,7 +154,7 @@
>>
>> private boolean isValidContentType(String type)
>> {
>> - return type.equals("text/html");
>> + return true;
>> }
>>
>> private String stripCharacterEncoding(String type)
>>
>
> --
> David Sean Taylor
> Bluesunrise Software
> david@bluesunrise.com
> [office] +01 707 773-4646
> [mobile] +01 707 529 9194
>
>
Re: Working on fix for bug PLUTO-55
Posted by David Sean Taylor <da...@bluesunrise.com>.
On Jul 8, 2004, at 4:13 PM, Eric Dalquist wrote:
> No, my first patch was a bit hasty, I didn't quite understand what the
> spec said when I wrote it. The workaround patch would be to take that
> method and simply return false in the body. I'll attach a CVS diff
> with this email. If you could incorporate this into CVS it would be
> very much appreciated.
>
patch applied
> Thank you,
> Eric Dalquist
>
> David Sean Taylor wrote:
>
>>
>> On Jul 8, 2004, at 12:38 PM, Eric Dalquist wrote:
>>
>>> Dave,
>>> Thank you for the reply and I would be happy to but I cannot
>>> figure out how to create the patch without making changes to quite a
>>> few method signatures so the RenderResponse has a reference to the
>>> RenderRequest. Without that reference I'm not sure how the
>>> RenderResponse is supposed to know what the valid content types
>>> returned by the getResponseContentType method on RenderRequest are.
>>> I can provide the code for the suggested interim fix which simply
>>> allows any content type to be set on the RenderResponse.
>>>
>> Is that the patch provided in your previous email?
>>
> Index: RenderResponseImpl.java
> ===================================================================
> RCS file:
> /home/cvspublic/jakarta-pluto/container/src/java/org/apache/pluto/
> core/impl/RenderResponseImpl.java,v
> retrieving revision 1.6
> diff -u -r1.6 RenderResponseImpl.java
> --- RenderResponseImpl.java 19 Mar 2004 12:40:21 -0000 1.6
> +++ RenderResponseImpl.java 8 Jul 2004 23:11:42 -0000
> @@ -154,7 +154,7 @@
>
> private boolean isValidContentType(String type)
> {
> - return type.equals("text/html");
> + return true;
> }
>
> private String stripCharacterEncoding(String type)
>
--
David Sean Taylor
Bluesunrise Software
david@bluesunrise.com
[office] +01 707 773-4646
[mobile] +01 707 529 9194
Re: Working on fix for bug PLUTO-55
Posted by Eric Dalquist <ed...@unicon.net>.
No, my first patch was a bit hasty, I didn't quite understand what the
spec said when I wrote it. The workaround patch would be to take that
method and simply return false in the body. I'll attach a CVS diff with
this email. If you could incorporate this into CVS it would be very much
appreciated.
Thank you,
Eric Dalquist
David Sean Taylor wrote:
>
> On Jul 8, 2004, at 12:38 PM, Eric Dalquist wrote:
>
>> Dave,
>> Thank you for the reply and I would be happy to but I cannot
>> figure out how to create the patch without making changes to quite a
>> few method signatures so the RenderResponse has a reference to the
>> RenderRequest. Without that reference I'm not sure how the
>> RenderResponse is supposed to know what the valid content types
>> returned by the getResponseContentType method on RenderRequest are. I
>> can provide the code for the suggested interim fix which simply
>> allows any content type to be set on the RenderResponse.
>>
> Is that the patch provided in your previous email?
>
Re: Working on fix for bug PLUTO-55
Posted by David Sean Taylor <da...@bluesunrise.com>.
On Jul 8, 2004, at 12:38 PM, Eric Dalquist wrote:
> Dave,
> Thank you for the reply and I would be happy to but I cannot figure
> out how to create the patch without making changes to quite a few
> method signatures so the RenderResponse has a reference to the
> RenderRequest. Without that reference I'm not sure how the
> RenderResponse is supposed to know what the valid content types
> returned by the getResponseContentType method on RenderRequest are. I
> can provide the code for the suggested interim fix which simply allows
> any content type to be set on the RenderResponse.
>
Is that the patch provided in your previous email?
Re: Working on fix for bug PLUTO-55
Posted by Eric Dalquist <ed...@unicon.net>.
Dave,
Thank you for the reply and I would be happy to but I cannot figure
out how to create the patch without making changes to quite a few method
signatures so the RenderResponse has a reference to the RenderRequest.
Without that reference I'm not sure how the RenderResponse is supposed
to know what the valid content types returned by the
getResponseContentType method on RenderRequest are. I can provide the
code for the suggested interim fix which simply allows any content type
to be set on the RenderResponse.
-Eric Dalquist
David Sean Taylor wrote:
>
> On Jul 7, 2004, at 12:31 PM, Eric Dalquist wrote:
>
>> Heh ... replying to my own message.
>>
>> Well after looking at the spec more my fix is incorrect. In
>> PLT.12.3.1 it is specified that when a portlet sets the content type
>> using setContentType on a RenderResponse the content type the portlet
>> is trying to set must be match, including wild cards, one of the
>> content types returned by the getResponseContentType in
>> RenderRequest. The problem is the RenderResponseImpl in pluto does
>> not have a reference to the RenderRequest. I'm not sure what needs to
>> be done to fix the problem to make this section of code compliant. In
>> the mean time could the isValidContentType be removed from
>> setContentType for a RenderResponse? This would allow us to use set
>> the content type as we should be able to.
>>
> Im also reviewing this.
> Could you provide a more complete patch meeting the requirements you
> described above?
> If so, i'd be glad to review and commit
>
> --
> David Sean Taylor
> Bluesunrise Software
> david@bluesunrise.com
> [office] +01 707 773-4646
> [mobile] +01 707 529 9194
>
>
Re: Working on fix for bug PLUTO-55
Posted by David Sean Taylor <da...@bluesunrise.com>.
On Jul 7, 2004, at 12:31 PM, Eric Dalquist wrote:
> Heh ... replying to my own message.
>
> Well after looking at the spec more my fix is incorrect. In PLT.12.3.1
> it is specified that when a portlet sets the content type using
> setContentType on a RenderResponse the content type the portlet is
> trying to set must be match, including wild cards, one of the content
> types returned by the getResponseContentType in RenderRequest. The
> problem is the RenderResponseImpl in pluto does not have a reference
> to the RenderRequest. I'm not sure what needs to be done to fix the
> problem to make this section of code compliant. In the mean time could
> the isValidContentType be removed from setContentType for a
> RenderResponse? This would allow us to use set the content type as we
> should be able to.
>
Im also reviewing this.
Could you provide a more complete patch meeting the requirements you
described above?
If so, i'd be glad to review and commit
--
David Sean Taylor
Bluesunrise Software
david@bluesunrise.com
[office] +01 707 773-4646
[mobile] +01 707 529 9194
Re: Working on fix for bug PLUTO-55
Posted by Eric Dalquist <ed...@unicon.net>.
Heh ... replying to my own message.
Well after looking at the spec more my fix is incorrect. In PLT.12.3.1
it is specified that when a portlet sets the content type using
setContentType on a RenderResponse the content type the portlet is
trying to set must be match, including wild cards, one of the content
types returned by the getResponseContentType in RenderRequest. The
problem is the RenderResponseImpl in pluto does not have a reference to
the RenderRequest. I'm not sure what needs to be done to fix the problem
to make this section of code compliant. In the mean time could the
isValidContentType be removed from setContentType for a RenderResponse?
This would allow us to use set the content type as we should be able to.
-Eric Dalquist
Eric Dalquist wrote:
> I'm working on a bug fix for
> http://nagoya.apache.org/jira/browse/PLUTO-55
>
> I've replaced the simple constant check for a "text/html" content type
> with the following (RenderResponseImpl.java):
>
> private boolean isValidContentType(String type)
> {
> PortletWindow window = this.getInternalPortletWindow();
> PortletEntity entity = window.getPortletEntity();
> PortletDefinition definition = entity.getPortletDefinition();
> ContentTypeSet contentTypes = definition.getContentTypeSet();
> //TODO: there appears to be code to see if a specific
> mode supports the
> // content type ... I need to figure out how to know what
> PortletMode
> // we are in at this point in the code.
> for (Iterator ctItr = contentTypes.iterator();
> ctItr.hasNext(); ) {
> ContentType ct = (ContentType)ctItr.next();
> if (ct.getContentType().equalsIgnoreCase(type)) {
> return true;
> }
> }
> return false;
> }
>
> This works and is better than what was there before but there is still
> a part missing. The ContentType should actually be checked against a
> PortletMode as well. ContentType has a supportsPortletMode |method
> which is passed a PortletMode and returns true or false if that
> ContentType is supported for the specified mode.
>
> I'm not sure how to get the current PortletMode in the
> RenderResponseImpl.
>
> Any suggestions would be much appreciated.
>
> -Eric Dalquist
> |