rombert commented on a change in pull request #24:
URL:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/24#discussion_r553440258
##########
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:
This method looks inconsistent to me - looks like it checks for valid
alias paths but returns true unless it gets a null argument, in which case it
throws an exception. Do we actually still need it?
##########
File path:
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
##########
@@ -100,6 +76,8 @@
private static final int VANITY_BLOOM_FILTER_MAX_ENTRIES = 10000000;
+ private static final Logger logger =
LoggerFactory.getLogger(MapEntries.class);
Review comment:
Either make this upper case or make it an instance field (instance field
is probably better from a class reload POV ).
##########
File path:
src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
##########
@@ -124,6 +114,15 @@ public void setup() throws Exception {
when(resourceResolverFactory.isMaxCachedVanityPathEntriesStartup()).thenReturn(true);
when(resourceResolver.findResources(anyString(),
eq("sql"))).thenReturn(
Collections.<Resource> emptySet().iterator());
+
//when(resourceResolverFactory.getAliasPath()).thenReturn(Arrays.asList("/child"));
+
+ Set<String> aliasPath = new TreeSet<>();
+ aliasPath.add("/parent");
+ for(int i = 1;i<testSize;i++){
Review comment:
Whitespace is off here compared to the rest of the class.
----------------------------------------------------------------
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]
