[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
[
https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17421640#comment-17421640
]
Mohit Arora commented on SLING-9620:
[~rombert] I have logged SLING-10844 with the explanation of the issue I am
seeing.
> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---
>
> Key: SLING-9620
> URL: https://issues.apache.org/jira/browse/SLING-9620
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.6.16
>Reporter: Angela Schreiber
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: Resource Resolver 1.7.0
>
> Attachments: SLING-9620-test.patch
>
> Time Spent: 3h 50m
> Remaining Estimate: 0h
>
> while investigating an issue involving {{sling:alias}}, i ended up manually
> adding the property using JCR API calls. this involved first adding the
> {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single
> or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
> mixin
>
> // alias name(s) for the node (single or multi-value)
> - sling:alias (string)
> - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that
> {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in
> {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216
> ({{String alias = ResourceResolverControl.getProperty(current,
> ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case
> just return a single string (it calls {{getProperty(res, propName,
> String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method
> will not get a complete list of all aliases available.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
[
https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17420527#comment-17420527
]
Robert Munteanu commented on SLING-9620:
[~mohiaror] - sounds complicated enough to be its own issue report :-) Can you
please create a new Jira issue for this?
> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---
>
> Key: SLING-9620
> URL: https://issues.apache.org/jira/browse/SLING-9620
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.6.16
>Reporter: Angela Schreiber
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: Resource Resolver 1.7.0
>
> Attachments: SLING-9620-test.patch
>
> Time Spent: 3h 50m
> Remaining Estimate: 0h
>
> while investigating an issue involving {{sling:alias}}, i ended up manually
> adding the property using JCR API calls. this involved first adding the
> {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single
> or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
> mixin
>
> // alias name(s) for the node (single or multi-value)
> - sling:alias (string)
> - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that
> {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in
> {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216
> ({{String alias = ResourceResolverControl.getProperty(current,
> ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case
> just return a single string (it calls {{getProperty(res, propName,
> String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method
> will not get a complete list of all aliases available.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
[
https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17420389#comment-17420389
]
Mohit Arora commented on SLING-9620:
[~rombert] It seems that after this bug fix, the behavior has changed for
ResourceMapperImpl.getMapping() when the resourcePath is an empty String. This
is happening because of the addition of [this
check|https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L157]
for mappedPath not being empty before adding it to mappings list. Earlier, we
used to add the mappedPath to mappings list irrespective of it being empty.
Because of that, for empty path, [getMapping function used to return a non null
value|https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L74]
but now it is returning null.
I agree that this was a bug with the earlier implementation and the callers of
getMapping API should check for null values being returned, but the [API
specifically
annotates|https://github.com/apache/sling-org-apache-sling-api/blob/master/src/main/java/org/apache/sling/api/resource/mapping/ResourceMapper.java#L96]
that for nonNull resourcePath, the return value will be nonNull. So in
"theory" this looks like a backwards incompatible change. Although checking the
implementation of ResourceMapperImpl the behavior is same from the beginning.
It seems the API was incorrectly annotated?
What should be done to address this? Should we correct the API annotation and
put Nullable as return value? I think that's something that needs to be done
anyway. Apart from that do you think we need to try to maintain backwards
compatibility here by even adding the empty resourcePath to the mappings list?
> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---
>
> Key: SLING-9620
> URL: https://issues.apache.org/jira/browse/SLING-9620
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.6.16
>Reporter: Angela Schreiber
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: Resource Resolver 1.7.0
>
> Attachments: SLING-9620-test.patch
>
> Time Spent: 3h 50m
> Remaining Estimate: 0h
>
> while investigating an issue involving {{sling:alias}}, i ended up manually
> adding the property using JCR API calls. this involved first adding the
> {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single
> or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
> mixin
>
> // alias name(s) for the node (single or multi-value)
> - sling:alias (string)
> - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that
> {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in
> {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216
> ({{String alias = ResourceResolverControl.getProperty(current,
> ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case
> just return a single string (it calls {{getProperty(res, propName,
> String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method
> will not get a complete list of all aliases available.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
[
https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17173123#comment-17173123
]
Robert Munteanu commented on SLING-9620:
[~angela] - yes, I will definitely look the optimisation enabled scenario as
well, and I want to test the same scenarios inside the unit test. It's going to
be a bit slower, but I think the test coverage is worth it.
> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---
>
> Key: SLING-9620
> URL: https://issues.apache.org/jira/browse/SLING-9620
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.6.16
>Reporter: Angela Schreiber
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: Resource Resolver 1.6.18
>
> Attachments: SLING-9620-test.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> while investigating an issue involving {{sling:alias}}, i ended up manually
> adding the property using JCR API calls. this involved first adding the
> {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single
> or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
> mixin
>
> // alias name(s) for the node (single or multi-value)
> - sling:alias (string)
> - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that
> {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in
> {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216
> ({{String alias = ResourceResolverControl.getProperty(current,
> ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case
> just return a single string (it calls {{getProperty(res, propName,
> String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method
> will not get a complete list of all aliases available.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
[
https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17173102#comment-17173102
]
Angela Schreiber commented on SLING-9620:
-
[~rombert], sure... i didn't invest a ton of time in the test case. just aimed
to have more at hand than just reporting something i spotted in Adobe AEM
without being able to illustrate it on the ResourceMapper level.
regarding the alias-optimization: since this is (as far as I know) enabled by
default in AEM, i think it would be really important to have it fixed as well.
i couldn't right way find the tests for that scenario. but it might be a good
thing to have additional test-coverage for the optimization-case, given the
fact that it is not recommended to disable it for performance reasons.
> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---
>
> Key: SLING-9620
> URL: https://issues.apache.org/jira/browse/SLING-9620
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.6.16
>Reporter: Angela Schreiber
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: Resource Resolver 1.6.18
>
> Attachments: SLING-9620-test.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> while investigating an issue involving {{sling:alias}}, i ended up manually
> adding the property using JCR API calls. this involved first adding the
> {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single
> or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
> mixin
>
> // alias name(s) for the node (single or multi-value)
> - sling:alias (string)
> - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that
> {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in
> {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216
> ({{String alias = ResourceResolverControl.getProperty(current,
> ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case
> just return a single string (it calls {{getProperty(res, propName,
> String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method
> will not get a complete list of all aliases available.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
[
https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17173085#comment-17173085
]
Robert Munteanu commented on SLING-9620:
[~angela] - thank your for the report and for the patch. I am getting close to
finalising the bug fix (at least for the optimise alias resolution disabled
scenario ).
To make the test advance I needed to make a small change to your patch, see
below:
{noformat}
diff --git
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java
index bc288e3..28a8c1b 100644
---
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java
+++
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java
@@ -19,7 +19,6 @@
package org.apache.sling.resourceresolver.impl.mapping;
-import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
@@ -67,7 +66,7 @@ public class InMemoryResourceProvider extends
ResourceProvider {
}
public void putResource(String path, String key, Object... values) {
-putResource(path, Collections.singletonMap(key,
Arrays.asList(values)));
+putResource(path, Collections.singletonMap(key, values));
}
public void putResource(String path, String key, Object value, String
key2, Object value2) {
{noformat}
Otherwise the property conversion will not work as expected. Does that look OK
to you?
> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---
>
> Key: SLING-9620
> URL: https://issues.apache.org/jira/browse/SLING-9620
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.6.16
>Reporter: Angela Schreiber
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: Resource Resolver 1.6.18
>
> Attachments: SLING-9620-test.patch
>
>
> while investigating an issue involving {{sling:alias}}, i ended up manually
> adding the property using JCR API calls. this involved first adding the
> {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single
> or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
> mixin
>
> // alias name(s) for the node (single or multi-value)
> - sling:alias (string)
> - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that
> {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in
> {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216
> ({{String alias = ResourceResolverControl.getProperty(current,
> ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case
> just return a single string (it calls {{getProperty(res, propName,
> String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method
> will not get a complete list of all aliases available.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
[
https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17167965#comment-17167965
]
Angela Schreiber commented on SLING-9620:
-
attached _SLING-9620-test.patch_ aims to illustrate the reported issue, which i
originally found when working with Adobe AEM. i hope, it's suited for some
initial validation of a possible fix.
Note: a fix will likely need to be made for the 2 cases, when _Optimize Alias
Resolution_ is enabled and disabled. I have not further tested the behavior of
{{ResourceMapper.getAllMappings}} with enabled optimization (as it adds another
layer of complexity) but from a first glance it might be that fixing this, will
surfaces additional issues.
> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---
>
> Key: SLING-9620
> URL: https://issues.apache.org/jira/browse/SLING-9620
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
>Affects Versions: Resource Resolver 1.6.16
>Reporter: Angela Schreiber
>Priority: Major
> Attachments: SLING-9620-test.patch
>
>
> while investigating an issue involving {{sling:alias}}, i ended up manually
> adding the property using JCR API calls. this involved first adding the
> {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single
> or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
> mixin
>
> // alias name(s) for the node (single or multi-value)
> - sling:alias (string)
> - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that
> {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in
> {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216
> ({{String alias = ResourceResolverControl.getProperty(current,
> ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case
> just return a single string (it calls {{getProperty(res, propName,
> String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method
> will not get a complete list of all aliases available.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
