[ 
https://issues.apache.org/jira/browse/CONFIGURATION-566?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13902882#comment-13902882
 ] 

Shen liang commented on CONFIGURATION-566:
------------------------------------------

I already use that fix in my current project and it works as expected.

Point1:
Yes, it does. Because inside the BeanUtils.setProperty(), it will check whether 
the caller is try to set <key, value> for the Map<>. The process on the map key 
value pair is different from the normal property. 

Point2:
Whether the property is writable or not, is handled by the 
BeanUtils.setProperty(), no need add another check just before it. If it is not 
writable, BeanUtils.setProperty() itself will throw the exception, with the 
detail error cause. No different from the current code. 

Step back, the current check function is using "PropertyUtils.isWriteable()", 
but actual job is done by "BeanUtils.setProperty()", apparently the logic is 
different since they are different function. 

To make my point2 more clearer, BeanUtils.setProperty() will throw the 
exception if really can't set the value. The error message is so similar!!!
{noformat}
BeanUtils.setProperty()
   call BeanUtilsBean.getInstance().setProperty()
          {
               ...
               // Invoke the setter method
        try {
          getPropertyUtils().setProperty(target, name, newValue);
        } catch (NoSuchMethodException e) {
            throw new InvocationTargetException
                (e, "Cannot set " + propName);
        } 
          }
{noformat}

BeanUtils.setProperty() will handle the map object differently.
{noformat}
BeanUtils.setProperty()
   call BeanUtilsBean.getInstance().setProperty()
         call getPropertyUtils().setProperty(target, name, newValue);
                call setNestedProperty()
                 {
                      ...
                      if (bean instanceof Map) {
                 }
{noformat}


> BeanHelper.createBean() can't support Map<> bean property loading from file
> ---------------------------------------------------------------------------
>
>                 Key: CONFIGURATION-566
>                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-566
>             Project: Commons Configuration
>          Issue Type: Bug
>          Components: Type conversion
>    Affects Versions: 1.10
>            Reporter: Shen liang
>
> The issue is BeanUtils.setProperty() can support the java Map bean to set the 
> (key, value) entry. But the BeahHelper.initProperty() add 1 more 
> PropertyUtils.isWriteable() check. While this PropertyUtils.isWriteable() 
> doesn't support java Map bean. 
> The check "PropertyUtils.isWriteable()" is quite redundant and unnecessary.
>   
> Is it better to remove the check "PropertyUtils.isWriteable()" since the 
> BeanUtils.setProperty() has various ways to set the properties?
> {noformat}
> BeanHelper.createBean() 
>  -> DefaultBeanFactory.createBean()
>       -> DefaultBeanFactory.initBeanInstance()
>            -> BeanHelper.initBean()
>                 ->BeahHelper.initProperty()
>                    {
>                         if (!PropertyUtils.isWriteable(bean, propName))
>                         ...
>                         BeanUtils.setProperty(bean, propName, value);
>                    }
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to