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 2023/01/09 15:13:36 UTC
[GitHub] [sling-org-apache-sling-resourceresolver] cristic83 opened a new pull request, #89: [SLING-11742] Provide alternative equitable terminology for properties
cristic83 opened a new pull request, #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89
This PR introduces alternative terminology for terms that are considered un-appropriate for vanity paths, as described in [1]. The approach taken was to provide additional properties to the existing properties so that backwards compatibility is kept. For the sake of backwards compatibility, the existing vanity paths properties are checked first and if these are not defined, the newly introduced properties are checked. A log message is produced in this case.
[1] https://issues.apache.org/jira/browse/SLING-11742
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cziegeler commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1574806488
@cristic83 I think we were a little bit unfair here with all our comments and nit picking. I think you did very good work so far. In order to spare you the burden of applying all our nitpicking, I put your code changes into a new branch and applied our comments there.
Created a new PR based on that branch https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/97
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "rombert (via GitHub)" <gi...@apache.org>.
rombert commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1453793552
I've assigned myself as a reviewer to make sure it's on my TODO list. Not sure when I will get around to it, if anyone beats me to it I won't mind :-)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cristic83 commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by GitBox <gi...@apache.org>.
cristic83 commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1066964951
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java:
##########
@@ -79,6 +81,45 @@ private static final class FactoryRegistration {
public volatile CommonResourceResolverFactoryImpl commonFactory;
}
+ private static final class VanityPathConfigurer {
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ void configureVanityPathPrefixes(String[] pathPrefixes, String[] pathPrefixesFallback,
Review Comment:
I attempted to implement this suggestion and the result looks like below. I don't think it's all that nicer,what do you think?
`void configureVanityPAthPrefix(ResourceResolverFactoryConfig config, Consumer<String[]> allowVanityPathConsumer, Consumer<String[]> denyVanityPathConsumer) {
configureVanityPathPrefixes(config.resource_resolver_vanitypath_whitelist(),
config.resource_resolver_vanitypath_allowlist(),
"resource_resolver_vanitypath_whitelist",
"resource_resolver_vanitypath_allowlist",
allowVanityPathConsumer);
configureVanityPathPrefixes(config.resource_resolver_vanitypath_blacklist(),
config.resource_resolver_vanitypath_denylist(),
"resource_resolver_vanitypath_blacklist",
"resource_resolver_vanitypath_denylist",
denyVanityPathConsumer);
}`
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SLING-11742] Provide alternative equitable terminology for properties [sling-org-apache-sling-resourceresolver]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler closed pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1712792090
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cziegeler commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1214089833
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java:
##########
@@ -163,15 +163,27 @@
@AttributeDefinition(name = "Allowed Vanity Path Location",
Review Comment:
we should remove the metatype info for this 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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1378783819
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cziegeler commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1214089318
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java:
##########
@@ -163,15 +163,27 @@
@AttributeDefinition(name = "Allowed Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
"such a list is configured, only vanity paths from resources starting with this prefix " +
- " are considered. If the list is empty, all vanity paths are used.")
+ " are considered. If the list is empty, we fallback to resource.resolver.vanitypath.allowlist.")
String[] resource_resolver_vanitypath_whitelist();
+ @AttributeDefinition(name = "Allowed Vanity Path Location",
+ description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
+ "such a list is configured, only vanity paths from resources starting with this prefix " +
+ " are considered. If the list is empty, all vanity paths are used.")
+ String[] resource_resolver_vanitypath_allowlist();
+
@AttributeDefinition(name = "Denied Vanity Path Location",
Review Comment:
We should remove the metatype information for this 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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1375931569
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list) [60.5% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by GitBox <gi...@apache.org>.
rombert commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1066899199
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java:
##########
@@ -163,15 +163,27 @@
@AttributeDefinition(name = "Allowed Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
"such a list is configured, only vanity paths from resources starting with this prefix " +
- " are considered. If the list is empty, all vanity paths are used.")
+ " are considered. If the list is empty, we fallback to resource_resolver_vanitypath_allowedlist.")
String[] resource_resolver_vanitypath_whitelist();
+ @AttributeDefinition(name = "Allowed Vanity Path Location",
+ description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
+ "such a list is configured, only vanity paths from resources starting with this prefix " +
+ " are considered. If the list is empty, all vanity paths are used.")
+ String[] resource_resolver_vanitypath_allowedlist();
+
@AttributeDefinition(name = "Denied Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /misc/. If " +
"such a list is configured,vanity paths from resources starting with this prefix " +
- " are not considered. If the list is empty, all vanity paths are used.")
+ " are not considered. If the list is empty, we fallback to resource_resolver_vanitypath_deniedlist.")
String[] resource_resolver_vanitypath_blacklist();
+ @AttributeDefinition(name = "Denied Vanity Path Location",
+ description ="This setting can contain a list of path prefixes, e.g. /misc/. If " +
+ "such a list is configured,vanity paths from resources starting with this prefix " +
+ " are not considered. If the list is empty, all vanity paths are used.")
+ String[] resource_resolver_vanitypath_deniedlist();
Review Comment:
Please use 'denylist' instead of 'deniedlist'
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java:
##########
@@ -163,15 +163,27 @@
@AttributeDefinition(name = "Allowed Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
"such a list is configured, only vanity paths from resources starting with this prefix " +
- " are considered. If the list is empty, all vanity paths are used.")
+ " are considered. If the list is empty, we fallback to resource_resolver_vanitypath_allowedlist.")
String[] resource_resolver_vanitypath_whitelist();
+ @AttributeDefinition(name = "Allowed Vanity Path Location",
+ description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
+ "such a list is configured, only vanity paths from resources starting with this prefix " +
+ " are considered. If the list is empty, all vanity paths are used.")
+ String[] resource_resolver_vanitypath_allowedlist();
Review Comment:
Please use 'allowlist' instead of 'allowedlist' . See also https://cwiki.apache.org/confluence/display/SLING/Removal+of+problematic+language
##########
src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java:
##########
@@ -195,6 +195,11 @@ public String[] resource_resolver_virtual() {
@Override
public String[] resource_resolver_vanitypath_whitelist() {
+ return resource_resolver_vanitypath_allowedlist();
Review Comment:
This can get confusing, please return just `null`.
##########
src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java:
##########
@@ -215,6 +220,11 @@ public int resource_resolver_vanitypath_bloomfilter_maxBytes() {
@Override
public String[] resource_resolver_vanitypath_blacklist() {
+ return resource_resolver_vanitypath_deniedlist();
Review Comment:
This can get confusing, please return just `null`.
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java:
##########
@@ -79,6 +81,45 @@ private static final class FactoryRegistration {
public volatile CommonResourceResolverFactoryImpl commonFactory;
}
+ private static final class VanityPathConfigurer {
Review Comment:
Please extract a test for this class, having the logic separated makes it easy.
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java:
##########
@@ -79,6 +81,45 @@ private static final class FactoryRegistration {
public volatile CommonResourceResolverFactoryImpl commonFactory;
}
+ private static final class VanityPathConfigurer {
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ void configureVanityPathPrefixes(String[] pathPrefixes, String[] pathPrefixesFallback,
+ String pathPrefixesPropertyName, String pathPrefixesFallbackPropertyName,
+ Consumer<String[]> filteredPathPrefixesConsumer) {
+ if (pathPrefixes != null) {
+ configureVanityPathPrefixes(pathPrefixes, filteredPathPrefixesConsumer);
Review Comment:
Please log a WARN in the case where both the regular prefixes and the fallback ones are defined; this is probably a customer error. We should rely on the old behaviour in this case and only take information from `config.resource_resolver_vanitypath_whitelist()`.
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java:
##########
@@ -79,6 +81,45 @@ private static final class FactoryRegistration {
public volatile CommonResourceResolverFactoryImpl commonFactory;
}
+ private static final class VanityPathConfigurer {
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ void configureVanityPathPrefixes(String[] pathPrefixes, String[] pathPrefixesFallback,
Review Comment:
I wonder if it makes sense to pass in the whole config object, so simplify the signature ; now we pass to fields and their names, passing in the object might make things nicer.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1717471449
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cziegeler commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1214092907
##########
src/main/java/org/apache/sling/resourceresolver/impl/VanityPathConfigurer.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.resourceresolver.impl;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class VanityPathConfigurer {
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ void configureVanityPathPrefixes(String[] pathPrefixes, String[] pathPrefixesFallback,
Review Comment:
This could be a static method just taking the Config object as a parameter and returning a String[]
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1398477174
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SLING-11742] Provide alternative equitable terminology for properties [sling-org-apache-sling-resourceresolver]
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1884294090
This has been implemented a while ago by #97
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cristic83 commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by GitBox <gi...@apache.org>.
cristic83 commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1066964951
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java:
##########
@@ -79,6 +81,45 @@ private static final class FactoryRegistration {
public volatile CommonResourceResolverFactoryImpl commonFactory;
}
+ private static final class VanityPathConfigurer {
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ void configureVanityPathPrefixes(String[] pathPrefixes, String[] pathPrefixesFallback,
Review Comment:
I attempted to implement this suggestion and the result looks like below. I don't think it's all that nicer,what do you think?
`void configureVanityPAthPrefix(ResourceResolverFactoryConfig config, Consumer<String[]> allowVanityPathConsumer,
Consumer<String[]> denyVanityPathConsumer) {
configureVanityPathPrefixes(config.resource_resolver_vanitypath_whitelist(),
config.resource_resolver_vanitypath_allowlist(),
"resource_resolver_vanitypath_whitelist",
"resource_resolver_vanitypath_allowlist",
allowVanityPathConsumer);
configureVanityPathPrefixes(config.resource_resolver_vanitypath_blacklist(),
config.resource_resolver_vanitypath_denylist(),
"resource_resolver_vanitypath_blacklist",
"resource_resolver_vanitypath_denylist",
denyVanityPathConsumer);
}`
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cristic83 commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by GitBox <gi...@apache.org>.
cristic83 commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1066964951
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java:
##########
@@ -79,6 +81,45 @@ private static final class FactoryRegistration {
public volatile CommonResourceResolverFactoryImpl commonFactory;
}
+ private static final class VanityPathConfigurer {
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ void configureVanityPathPrefixes(String[] pathPrefixes, String[] pathPrefixesFallback,
Review Comment:
I attempted to implement this suggestion and the result looks like below. I don't think it's all that nicer,what do you think?
`
void configureVanityPAthPrefix(ResourceResolverFactoryConfig config, Consumer<String[]> allowVanityPathConsumer, Consumer<String[]> denyVanityPathConsumer) {
configureVanityPathPrefixes(config.resource_resolver_vanitypath_whitelist(),
config.resource_resolver_vanitypath_allowlist(),
"resource_resolver_vanitypath_whitelist",
"resource_resolver_vanitypath_allowlist",
allowVanityPathConsumer);
configureVanityPathPrefixes(config.resource_resolver_vanitypath_blacklist(),
config.resource_resolver_vanitypath_denylist(),
"resource_resolver_vanitypath_blacklist",
"resource_resolver_vanitypath_denylist",
denyVanityPathConsumer);
}
`
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1514967267
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cziegeler commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1465756017
@cristic83 I suggest we take a similar approach to https://github.com/apache/sling-org-apache-sling-jcr-base/pull/9 where we have let the code just use the new properties, but have a conversion from old to new
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] cziegeler commented on a diff in pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on code in PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#discussion_r1214088620
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java:
##########
@@ -163,15 +163,27 @@
@AttributeDefinition(name = "Allowed Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
"such a list is configured, only vanity paths from resources starting with this prefix " +
- " are considered. If the list is empty, all vanity paths are used.")
+ " are considered. If the list is empty, we fallback to resource.resolver.vanitypath.allowlist.")
Review Comment:
I think the fallback should be in the opposite direction: if the new property is not used, the old one will be used
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java:
##########
@@ -163,15 +163,27 @@
@AttributeDefinition(name = "Allowed Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
"such a list is configured, only vanity paths from resources starting with this prefix " +
- " are considered. If the list is empty, all vanity paths are used.")
+ " are considered. If the list is empty, we fallback to resource.resolver.vanitypath.allowlist.")
String[] resource_resolver_vanitypath_whitelist();
+ @AttributeDefinition(name = "Allowed Vanity Path Location",
+ description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
+ "such a list is configured, only vanity paths from resources starting with this prefix " +
+ " are considered. If the list is empty, all vanity paths are used.")
+ String[] resource_resolver_vanitypath_allowlist();
+
@AttributeDefinition(name = "Denied Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /misc/. If " +
"such a list is configured,vanity paths from resources starting with this prefix " +
- " are not considered. If the list is empty, all vanity paths are used.")
+ " are not considered. If the list is empty, we fallback to resource.resolver.vanitypath.denylist.")
Review Comment:
same as above
--
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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented on pull request #89: [SLING-11742] Provide alternative equitable terminology for properties
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #89:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/89#issuecomment-1497714921
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=89&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.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org