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 Zhong ZHENG <he...@gmail.com> on 2005/09/13 18:28:01 UTC

Bug report for Pluto 1.1: creation of PreferencesValidator breaks the portlet spec.

 The portlet spec states (PLT 14.4): 

 If a portlet definition includes a validator, the portlet container must 
create a single validator instance per portlet definition. If the 
application is a distributed application, the portlet container must create 
an instance per VM. 

While in the current implementation (Branch pluto-1.1, 
PortletPreferencesImpl.store() ), every time the method store() is called, a 
new PreferencesValidator instance is created. The validator should have been 
associated with the portlet definition. 

I created an issue for this: http://issues.apache.org/jira/browse/PLUTO-166

I also tried to fix this problem by modifying the method 
PortletPreferencesDD.getPreferencesValidator() in the descriptor-api, as the 
following: 

/**
 * Hold the PreferencesValidator instance,
 * to ensure that for each portlet definition,
 * only one validator instance is created.
 */
 private PreferencesValidator validatorInstance = null; 

/**
 * Get the PreferencesValidator instance instead of
 * the class name.
 */
 public PreferencesValidator getPreferencesValidator() 
throws InstantiationException, IllegalAccessException,
ClassNotFoundException, ClassCastException { 
if (validatorInstance == null && preferencesValidator != null) { 
ClassLoader loader = Thread.currentThread().getContextClassLoader(); 
Class clazz = loader.loadClass(preferencesValidator); 
validatorInstance = (PreferencesValidator) clazz.newInstance(); 
} 
return validatorInstance; 
} 

And I updated the PortletPreferencesImpl accordingly. That works, but is not 
a good solution. I think the descriptor-api should hold only domain objects 
created from portlet.xml, and doesn't need to know the portlet API (such as 
javax.portlet.PreferencesValidator interface). 

I am thinking to add a wrapper class for PortletDD, such as PortletDDWrapper, 
in the container subproject. This class may hold some additional information 
about the portlet definition (for example, it may hold an instance of 
PreferencesValidator, to ensure that for each portlet definition, only one 
validator is created). Then in the container, instead of using
PortletDDdirectly, we may use
PortletDDWrapper through which we may get the validator. 

If I update the source like this, quite a few classes need to be modified. 
So I would like to listen to your opinions first. 

Regards. 

 -- 

ZHENG Zhong

1 Avenue Alphand
75116 Paris, France
+33 6 76 80 45 90

heavyzheng {AT} gmail {D0T} com

http://heavyz.sourceforge.net
http://heavyz.blogspot.com
http://spaces.msn.com/members/zhengzhong

Re: Bug report for Pluto 1.1: creation of PreferencesValidator breaks the portlet spec.

Posted by "David H. DeWolf" <dd...@apache.org>.
oops, helps if I finish my thought before sending. . .   :)

see below.


David H. DeWolf wrote:
> 
> 
> Zhong ZHENG wrote:
> 
>>   The portlet spec states (PLT 14.4):
>>
>> If a portlet definition includes a validator, the portlet container 
>> must create a single validator instance per portlet definition. If the 
>> application is a distributed application, the portlet container must 
>> create an instance per VM.
>>
>> While in the current implementation (Branch pluto-1.1, 
>> PortletPreferencesImpl.store() ), every time the method store() is 
>> called, a new PreferencesValidator instance is created. The validator 
>> should have been associated with the portlet definition.
>>
>> I created an issue for this: 
>> http://issues.apache.org/jira/browse/PLUTO-166
>>
>> I also tried to fix this problem by modifying the method 
>> PortletPreferencesDD.getPreferencesValidator() in the descriptor-api, 
>> as the following:
>>         /**
>>      * Hold the PreferencesValidator instance,
>>      * to ensure that for each portlet definition,
>>      * only one validator instance is created.
>>      */
>>     private PreferencesValidator validatorInstance = null;
>>        /**
>>      * Get the PreferencesValidator instance instead of
>>      * the class name.
>>      */
>>     public PreferencesValidator getPreferencesValidator()
>>     throws InstantiationException, IllegalAccessException,
>>             ClassNotFoundException, ClassCastException {
>>         if (validatorInstance == null && preferencesValidator != null) {
>>             ClassLoader loader = 
>> Thread.currentThread().getContextClassLoader();
>>             Class clazz = loader.loadClass(preferencesValidator);
>>             validatorInstance = (PreferencesValidator) 
>> clazz.newInstance();
>>         }
>>         return validatorInstance;
>>     }
>>
>> And I updated the PortletPreferencesImpl accordingly. That works, but 
>> is not a good solution. I think the descriptor-api should hold only 
>> domain objects created from portlet.xml, and doesn't need to know the 
>> portlet API (such as javax.portlet.PreferencesValidator interface).
> 
> 
> I agree, the domain objects should be simple beans with no external 
> dependencies (including the portlet api).
> 
>>
>> I am thinking to add a wrapper class for PortletDD, such as 
>> PortletDDWrapper, in the container subproject. This class may hold 
>> some additional information about the portlet definition (for example, 
>> it may hold an instance of PreferencesValidator, to ensure that for 
>> each portlet definition, only one validator is created). Then in the 
>> container, instead of using PortletDD directly, we may use 
>> PortletDDWrapper through which we may get the validator.
> 
> 
> Check out the portlet entity instead of creating a new portlet dd 
> wrapper.  The entity allready contains business logic like you are 
> describing - specifically, it caches things like the .

What I meant to say is that it allready  caches things like portlet 
preferences.

David

> 
>>
>> If I update the source like this, quite a few classes need to be 
>> modified. So I would like to listen to your opinions first.
> 
> 
> If you like my idea about adding it to the portlet entity - go for it!
> 
>>
>> Regards.
>>
>> -- 
>>
>> ZHENG Zhong
>>
>> 1 Avenue Alphand
>> 75116 Paris, France
>> +33 6 76 80 45 90
>>
>> heavyzheng {AT} gmail {D0T} com
>>
>> http://heavyz.sourceforge.net <http://heavyz.sourceforge.net>
>> http://heavyz.blogspot.com <http://heavyz.blogspot.com>
>> http://spaces.msn.com/members/zhengzhong 
>> <http://spaces.msn.com/members/zhengzhong>
>>
>>
> 
> 


Re: Bug report for Pluto 1.1: creation of PreferencesValidator breaks the portlet spec.

Posted by "David H. DeWolf" <dd...@apache.org>.

Zhong ZHENG wrote:
>   The portlet spec states (PLT 14.4):
> 
> If a portlet definition includes a validator, the portlet container must 
> create a single validator instance per portlet definition. If the 
> application is a distributed application, the portlet container must 
> create an instance per VM.
> 
> While in the current implementation (Branch pluto-1.1, 
> PortletPreferencesImpl.store() ), every time the method store() is 
> called, a new PreferencesValidator instance is created. The validator 
> should have been associated with the portlet definition.
> 
> I created an issue for this: http://issues.apache.org/jira/browse/PLUTO-166
> 
> I also tried to fix this problem by modifying the method 
> PortletPreferencesDD.getPreferencesValidator() in the descriptor-api, as 
> the following:
>     
>     /**
>      * Hold the PreferencesValidator instance,
>      * to ensure that for each portlet definition,
>      * only one validator instance is created.
>      */
>     private PreferencesValidator validatorInstance = null;
>    
>     /**
>      * Get the PreferencesValidator instance instead of
>      * the class name.
>      */
>     public PreferencesValidator getPreferencesValidator()
>     throws InstantiationException, IllegalAccessException,
>             ClassNotFoundException, ClassCastException {
>         if (validatorInstance == null && preferencesValidator != null) {
>             ClassLoader loader = 
> Thread.currentThread().getContextClassLoader();
>             Class clazz = loader.loadClass(preferencesValidator);
>             validatorInstance = (PreferencesValidator) clazz.newInstance();
>         }
>         return validatorInstance;
>     }
> 
> And I updated the PortletPreferencesImpl accordingly. That works, but is 
> not a good solution. I think the descriptor-api should hold only domain 
> objects created from portlet.xml, and doesn't need to know the portlet 
> API (such as javax.portlet.PreferencesValidator interface).

I agree, the domain objects should be simple beans with no external 
dependencies (including the portlet api).

> 
> I am thinking to add a wrapper class for PortletDD, such as 
> PortletDDWrapper, in the container subproject. This class may hold some 
> additional information about the portlet definition (for example, it may 
> hold an instance of PreferencesValidator, to ensure that for each 
> portlet definition, only one validator is created). Then in the 
> container, instead of using PortletDD directly, we may use 
> PortletDDWrapper through which we may get the validator.

Check out the portlet entity instead of creating a new portlet dd 
wrapper.  The entity allready contains business logic like you are 
describing - specifically, it caches things like the .
> 
> If I update the source like this, quite a few classes need to be 
> modified. So I would like to listen to your opinions first.

If you like my idea about adding it to the portlet entity - go for it!
> 
> Regards.
> 
> -- 
> 
> ZHENG Zhong
> 
> 1 Avenue Alphand
> 75116 Paris, France
> +33 6 76 80 45 90
> 
> heavyzheng {AT} gmail {D0T} com
> 
> http://heavyz.sourceforge.net <http://heavyz.sourceforge.net>
> http://heavyz.blogspot.com <http://heavyz.blogspot.com>
> http://spaces.msn.com/members/zhengzhong 
> <http://spaces.msn.com/members/zhengzhong>
> 
>