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

Justin Edelson commented on SLING-7187:
---------------------------------------

bq. The patch in general looks fine but it would modify the behavior in case 
both the optional attribute is set to false as well as the @Optional annotation 
is set, as with your patch the @Optional would take precedence. But since this 
would be rather unexpected I don't think this is a problem. 

Yes, that's the whole point :) since the optional attribute is set to false by 
default.

bq. I would just like to highlight that the optional attribute is anyhow 
deprecated since SLING-4155 and instead the injectionStrategy attribute should 
be used.

The problem is that even if someone doesn't use it, it is still set. I guess 
the other alternative to this would be to remove support for the optional 
attribute entirely (i.e. just ignore it if set). I don't think we need to do 
that.

> Use of Injector-specific annotation and @Optional results in required-value
> ---------------------------------------------------------------------------
>
>                 Key: SLING-7187
>                 URL: https://issues.apache.org/jira/browse/SLING-7187
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: Sling Models Impl 1.4.4
>            Reporter: Justin Edelson
>             Fix For: Sling Models Impl 1.4.6
>
>         Attachments: SLING-7187.diff
>
>
> If there is a model class like
> {code}
>     @Model(adaptables = Resource.class)
>     public class ModelClass {
>         @ValueMapValue
>         private String otherText;
>         @Override
>         public String getOtherText() {
>             return otherText;
>         }
>         @ValueMapValue
>         @Optional
>         private String emptyText;
>         @Override
>         public String getEmptyText() {
>             return emptyText;
>         }
>     }
> {code}
> It would be a reasonable expectation that the {{emptyText}} field will be 
> optional. But that is not actually the case since the {{@ValueMapValue}} has 
> a default {{optional}} attribute of {{false}}.
> Can be worked around by using {{@ValueMapValue(optional = true)}} instead of 
> having the separate {{@Optional}} annotation.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to