You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2020/03/05 13:44:06 UTC
[GitHub] [sling-org-apache-sling-dynamic-include] mittalxkunal opened a new
pull request #14: SLING-9174: Added the code for enhancement
mittalxkunal opened a new pull request #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14
Enhancement: https://issues.apache.org/jira/browse/SLING-9174
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] jfmitchell commented on a
change in pull request #14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
jfmitchell commented on a change in pull request #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#discussion_r388544309
##########
File path: src/main/java/org/apache/sling/dynamicinclude/IncludeTagFilter.java
##########
@@ -166,8 +167,22 @@ private String getUrl(Configuration config, SlingHttpServletRequest request) {
private String buildUrl(Configuration config, SlingHttpServletRequest request) {
final Resource resource = request.getResource();
+ // The below code gets the path to the XF and then passes it to the buildUrl method
+ // so that the path to the component is replaced with path to the XF
+ ValueMap vm = resource.adaptTo(ValueMap.class);
Review comment:
Yes - but this code is going to run for *all* configured resource types.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] mittalxkunal commented on
a change in pull request #14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
mittalxkunal commented on a change in pull request #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#discussion_r388338105
##########
File path: src/main/java/org/apache/sling/dynamicinclude/IncludeTagFilter.java
##########
@@ -166,8 +167,22 @@ private String getUrl(Configuration config, SlingHttpServletRequest request) {
private String buildUrl(Configuration config, SlingHttpServletRequest request) {
final Resource resource = request.getResource();
+ // The below code gets the path to the XF and then passes it to the buildUrl method
+ // so that the path to the component is replaced with path to the XF
+ ValueMap vm = resource.adaptTo(ValueMap.class);
Review comment:
Do we need to check for resource type because its already checked before it comes to buildUrl method based on the configuration added in resource-types OSGi config
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] sonarcloud[bot] commented
on issue #14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on issue #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#issuecomment-595641025
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/40.png' alt='45.2%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_coverage&view=list) [45.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] sonarcloud[bot] removed a
comment on issue #14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on issue #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#issuecomment-595641025
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/40.png' alt='45.2%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_coverage&view=list) [45.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] rombert commented on issue
#14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
rombert commented on issue #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#issuecomment-610265262
@toniedzwiedz - would be great to have your input as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] sonarcloud[bot] commented
on issue #14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on issue #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#issuecomment-596057631
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/40.png' alt='45.2%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_coverage&view=list) [45.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-dynamic-include&pullRequest=14&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] jfmitchell commented on a
change in pull request #14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
jfmitchell commented on a change in pull request #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#discussion_r388305344
##########
File path: src/main/java/org/apache/sling/dynamicinclude/IncludeTagFilter.java
##########
@@ -166,8 +167,22 @@ private String getUrl(Configuration config, SlingHttpServletRequest request) {
private String buildUrl(Configuration config, SlingHttpServletRequest request) {
final Resource resource = request.getResource();
+ // The below code gets the path to the XF and then passes it to the buildUrl method
+ // so that the path to the component is replaced with path to the XF
+ ValueMap vm = resource.adaptTo(ValueMap.class);
Review comment:
I would base the code on the resource type rather than the existence of a property.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-dynamic-include] mittalxkunal commented on
a change in pull request #14: SLING-9174: Added the code for enhancement
Posted by GitBox <gi...@apache.org>.
mittalxkunal commented on a change in pull request #14: SLING-9174: Added the code for enhancement
URL: https://github.com/apache/sling-org-apache-sling-dynamic-include/pull/14#discussion_r403326356
##########
File path: src/main/java/org/apache/sling/dynamicinclude/IncludeTagFilter.java
##########
@@ -166,8 +167,22 @@ private String getUrl(Configuration config, SlingHttpServletRequest request) {
private String buildUrl(Configuration config, SlingHttpServletRequest request) {
final Resource resource = request.getResource();
+ // The below code gets the path to the XF and then passes it to the buildUrl method
+ // so that the path to the component is replaced with path to the XF
+ ValueMap vm = resource.adaptTo(ValueMap.class);
Review comment:
So within the config you can configure multiple resource types and then enable option to rewrite content path instead of component path. So this will be enabled for all configured resource types...
If there was a 1:1 mapping then we would verify for which resourcetype path rewrite is enabled... Does that make sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services