rombert commented on a change in pull request #24:
URL:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/24#discussion_r553864673
##########
File path:
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
##########
@@ -1038,22 +1016,69 @@ private boolean addEntry(final Map<String,
List<MapEntry>> entryMap, final Strin
*/
private Map<String, Map<String, String>> loadAliases(final
ResourceResolver resolver) {
final Map<String, Map<String, String>> map = new ConcurrentHashMap<>();
- final String queryString = ALIAS_QUERY_DEFAULT;
- final Iterator<Resource> i =
resolver.findResources(queryString, "sql");
- while (i.hasNext()) {
+ final String queryString = updateAliasQuery();
+ final Iterator<Resource> i = resolver.findResources(queryString,
"sql");
+ while (i.hasNext()) {
final Resource resource = i.next();
loadAlias(resource, map);
}
return map;
}
- /**
+
+ /*
+ * Update alias query based on configured alias locations
+ */
+ private String updateAliasQuery(){
+ final Set<String> allowedLocations =
this.factory.getAllowedAliasLocations();
+
+ StringBuilder baseQuery = new StringBuilder(ALIAS_BASE_QUERY_DEFAULT);
+ baseQuery.append(" ").append("WHERE");
+
+ if(allowedLocations.isEmpty()){
+ baseQuery.append(" ").append("(").append("NOT
ISDESCENDANTNODE(page,")
+
.append("\"").append(JCR_SYSTEM_PREFIX).append("\"").append("))");
+
+ }else{
+ Iterator<String> pathIterator = allowedLocations.iterator();
+ baseQuery.append("(");
+ while(pathIterator.hasNext()){
+ String prefix = pathIterator.next();
+ baseQuery.append(" ").append("ISDESCENDANTNODE(page,")
+ .append("\"").append(prefix).append("\"")
+ .append(")").append(" ").append("OR");
+ }
+ //Remove last "OR" keyword
+ int orLastIndex = baseQuery.lastIndexOf("OR");
+ baseQuery.delete(orLastIndex,baseQuery.length());
+ baseQuery.append(")");
+ }
+
+ baseQuery.append(" AND sling:alias IS NOT NULL ");
+ String aliasQuery = baseQuery.toString();
+ logger.debug("Query to fetch alias [{}] ", aliasQuery);
+
+ return aliasQuery;
+ }
+
+ /**
+ *
+ * validate alias path based on configuration provided
+ */
+ protected boolean isValidAliasPath(final String path){
Review comment:
Yes, I saw that. I was questioning if we really need it. If we do, the
name is not correct.
##########
File path:
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
##########
@@ -1038,22 +1016,69 @@ private boolean addEntry(final Map<String,
List<MapEntry>> entryMap, final Strin
*/
private Map<String, Map<String, String>> loadAliases(final
ResourceResolver resolver) {
final Map<String, Map<String, String>> map = new ConcurrentHashMap<>();
- final String queryString = ALIAS_QUERY_DEFAULT;
- final Iterator<Resource> i =
resolver.findResources(queryString, "sql");
- while (i.hasNext()) {
+ final String queryString = updateAliasQuery();
+ final Iterator<Resource> i = resolver.findResources(queryString,
"sql");
+ while (i.hasNext()) {
final Resource resource = i.next();
loadAlias(resource, map);
}
return map;
}
- /**
+
+ /*
+ * Update alias query based on configured alias locations
+ */
+ private String updateAliasQuery(){
+ final Set<String> allowedLocations =
this.factory.getAllowedAliasLocations();
+
+ StringBuilder baseQuery = new StringBuilder(ALIAS_BASE_QUERY_DEFAULT);
+ baseQuery.append(" ").append("WHERE");
+
+ if(allowedLocations.isEmpty()){
+ baseQuery.append(" ").append("(").append("NOT
ISDESCENDANTNODE(page,")
+
.append("\"").append(JCR_SYSTEM_PREFIX).append("\"").append("))");
+
+ }else{
+ Iterator<String> pathIterator = allowedLocations.iterator();
+ baseQuery.append("(");
+ while(pathIterator.hasNext()){
+ String prefix = pathIterator.next();
+ baseQuery.append(" ").append("ISDESCENDANTNODE(page,")
+ .append("\"").append(prefix).append("\"")
+ .append(")").append(" ").append("OR");
+ }
+ //Remove last "OR" keyword
+ int orLastIndex = baseQuery.lastIndexOf("OR");
+ baseQuery.delete(orLastIndex,baseQuery.length());
+ baseQuery.append(")");
+ }
+
+ baseQuery.append(" AND sling:alias IS NOT NULL ");
+ String aliasQuery = baseQuery.toString();
+ logger.debug("Query to fetch alias [{}] ", aliasQuery);
+
+ return aliasQuery;
+ }
+
+ /**
+ *
+ * validate alias path based on configuration provided
+ */
+ protected boolean isValidAliasPath(final String path){
Review comment:
Right.
What I am saying is that it never returns false, so checking the return
value is not useful. It should be something like `assertValidAliasPath` or
`checkValidAliasPath`, have a void return type and only throw the exception.
----------------------------------------------------------------
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:
[email protected]
