[jira] [Commented] (SLING-7187) Use of Injector-specific annotation and @Optional results in required-value
[
https://issues.apache.org/jira/browse/SLING-7187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199005#comment-16199005
]
Justin Edelson commented on SLING-7187:
---
I did find one additional oddity.
{code}
@ValueMapValue(optional=true)
@Required
{code}
In this case, the field is optional. BUT...
{code}
@ValueMapValue(injectionStrategy = InjectionStrategy.OPTIONAL)
@Required
{code}
The field is optional.
Since, as [~kwin] notes the optional attribute is deprecated, I'm not going to
fix that, but I did add the test cases to save the behavior.
> 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)
[jira] [Commented] (SLING-7187) Use of Injector-specific annotation and @Optional results in required-value
[
https://issues.apache.org/jira/browse/SLING-7187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)
[jira] [Commented] (SLING-7187) Use of Injector-specific annotation and @Optional results in required-value
[
https://issues.apache.org/jira/browse/SLING-7187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16198900#comment-16198900
]
Konrad Windszus commented on SLING-7187:
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.
I would just like to highlight that the optional attribute is anyhow deprecated
since SLING-4155 and instead the injectionStrategy attribute should be used.
> 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)
