[jira] [Comment Edited] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations

["Konrad Windszus (JIRA)" <ji...@apache.org>]

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

Konrad Windszus edited comment on SLING-4155 at 11/13/14 3:01 PM:
------------------------------------------------------------------

In my pull-request I already raised the exported package version number to 2.0 (thanks to the new baseline feature of the maven-bundle-plugin).

I don't want that feature to be broken until we release 2.0 so what about the following approach:
# add a *new* field to all injector-specific annotations named {{isOptional}} with the default value {{DEFAULT}} (following the default injection strategy)
# deprecate the old field {{optional}} (which still has {{false}} as its default value)

With v2.0 of Sling Models we could then finally remove the field {{optional}}

Now the default injection strategy should be followed if
# {{isOptional = DEFAULT}} and
# optional = false (either explicitly set or just because it is the default - it is impossible to distinguish those two cases)

Now if we look at the following usecases this is the new vs. the old behaviour:

||optional||DefaultInjectionStrategy||old behaviour||new behaviour||
|{{false}}(default)|{{Required}}(default)|required|required|
|{{false}}(default)|{{Optional}}|required|optional|
|{{true}}|{{Required}}(default)|optional|optional|
|{{true}}|{{Optional}}|optional|optional|

Although this actually changes the second of the four usecases I don't think this will ever affect someone, because the DefaultInjectionStrategy was probably never set on models using injector-specific annotations (due to this bug).
WDYT?


was (Author: kwin):
In my pull-request I already raised the exported package version number to 2.0 (thanks to the new baseline feature of the maven-bundle-plugin).

I don't want that feature to be broken until we release 2.0 so what about the following approach:
# add a *new* field to all injector-specific annotations named {{isOptional}} with the default value {{DEFAULT}} (following the default injection strategy)
# deprecate the old field {{optional}} (which still has {{false}} as its default value)

With v2.0 of Sling Models we could then finally remove the field {{optional}}

Now the default injection strategy should be followed if
# {{isOptional = DEFAULT}}
# optional = false (either explicitly set or just because it is the default - it is impossible to distinguish those two cases)

Now if we look at the following usecases this is the new vs. the old behaviour:

||optional||DefaultInjectionStrategy||old behaviour||new behaviour||
|{{false}}(default)|{{Required}}(default)|required|required|
|{{false}}(default)|{{Optional}}|required|optional|
|{{true}}|{{Required}}(default)|optional|optional|
|{{true}}|{{Optional}}|optional|optional|

Although this actually changes the second of the four usecases I don't think this will ever affect someone, because the DefaultInjectionStrategy was probably never set on models using injector-specific annotations (due to this bug).
WDYT?

> DefaultInjectionStrategy not considered for injector-specific annotations
> -------------------------------------------------------------------------
>
>                 Key: SLING-4155
>                 URL: https://issues.apache.org/jira/browse/SLING-4155
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: Sling Models Implementation 1.0.6
>            Reporter: Konrad Windszus
>
> The default injection strategy (being implemented in SLING-3696) is only considered, in case there is no injector-specific annotation being used.
> Otherwise it is just ignored.
> The logic should be like this:
> if annotationProcessor.isOptional() returns null
> -> the default injection strategy should be used
> in any other case the boolean value should be used.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)