[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[ https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14539812#comment-14539812 ] Stefan Seifert commented on SLING-4155: --- Completed: At revision: 1678937 fixed a typo in an annotation attribute: injectonStrategy => injectionStrategy > 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 >Assignee: Konrad Windszus > Fix For: Sling Models API 1.2.0, Sling Models Impl 1.2.0 > > > 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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[ https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14224466#comment-14224466 ] ASF GitHub Bot commented on SLING-4155: --- Github user kwin closed the pull request at: https://github.com/apache/sling/pull/33 > 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 >Assignee: Konrad Windszus > Fix For: Sling Models API 1.2.0, Sling Models Impl 1.2.0 > > > 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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[
https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14221015#comment-14221015
]
Konrad Windszus commented on SLING-4155:
[~sseif...@pro-vision.de] Thanks for the review. I just deprecated
{{InjectAnnotationProcessorFactory}} in rev. 1640931.
> 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
>Assignee: Konrad Windszus
> Fix For: Sling Models API 1.2.0, Sling Models Impl 1.2.0
>
>
> 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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[ https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14219201#comment-14219201 ] Konrad Windszus commented on SLING-4155: I committed the changes in rev. 1640710. > 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 >Assignee: Konrad Windszus > Fix For: Sling Models API 1.2.0, Sling Models Impl 1.2.0 > > > 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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[ https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14216473#comment-14216473 ] Konrad Windszus commented on SLING-4155: I updated the PR (https://github.com/apache/sling/pull/33). [~sseif...@pro-vision.de],[~justinedelson] Can you both have a look at that? > 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 >Assignee: 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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[
https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14214894#comment-14214894
]
Konrad Windszus commented on SLING-4155:
[~sseif...@pro-vision.de] Thanks for your input. I agree that
{{injectionStrategy}} is a better name. I would still need to increase the
version to 2.0.0 on {{org.apache.sling.models.spi.injectorspecific}}, because I
would need to add the {{getInjectionStrategy()}} method on the interface
{{InjectAnnotationProcessor}}. To make that backwards compatible one would need
to add an additional interface {{InjectAnnotationProcessor2}} which could then
have a method to retrieve the value from {{injectionStrategy}}.
That will still support the old {{InjectAnnotationProcessor}} with the known
limitations. But for all annotations being supported by Sling Models I could
implement the new interface, leveraging that additional field on the
annotation. 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
>Assignee: 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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[
https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14213666#comment-14213666
]
Stefan Seifert commented on SLING-4155:
---
yes, from my perspective that would be a good way.
the only drawback are the to annotation fields optional and isOptional - but
that could be documented in the API.
an alternative would be another name like {{injectionStrategy}}, with values
INHERIT, OPTIONAL, REQUIRED. this would be closer to the naming of
defaultInjectionStrategy of the @Model annotation.
> 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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[
https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14209860#comment-14209860
]
Konrad Windszus commented on SLING-4155:
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)
[jira] [Commented] (SLING-4155) DefaultInjectionStrategy not considered for injector-specific annotations
[ https://issues.apache.org/jira/browse/SLING-4155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14209804#comment-14209804 ] Stefan Seifert commented on SLING-4155: --- such a change on the annotations will indeed break both runtime and compile-time backward compatibility - we cannot do this unless we raise the version to v2.0.0 to comply to semantic versioning. i'm not sure if it's worth only for this issue, although i agree the current functionality is broken when injector-specific annotations are used. for a first step we should document this issue (SLING-4156), and perhaps create a new version in JIRA to sling models 2.0.0 and collect there tickets like this so we do not forget it when time is ready for v2. > 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)
