henrykuijpers commented on a change in pull request #46:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/46#discussion_r644635245



##########
File path: 
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java
##########
@@ -112,11 +112,6 @@
                       "the ResourceResolver mapping. The default value is 
/etc/map.")
     String resource_resolver_map_location() default 
MapEntries.DEFAULT_MAP_ROOT;
 
-    @AttributeDefinition(name = "Mapping Observation",

Review comment:
       The mapping observation configuration setting is used to register the 
`ResourceChangeListener`, so that the `MapEntries` will be notified of changes 
that it is interested in. In AEM, it is set to `/`. 
   
   However, given the various configurations that exist, it is easy to compute 
the right paths to use. (That is also changed in the part where the listener is 
registered.)
   
   There are also some fallbacks in there to listen to `/` anyway, if that is 
required.

##########
File path: 
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
##########
@@ -1255,6 +1279,33 @@ private boolean loadVanityPath(final Resource resource, 
final Map<String, List<M
         return hasVanityPath;
     }
 
+    private String createVanityPathQuery() {
+        final String query =
+            VANITY_PATH_BASE_QUERY_DEFAULT +
+                " WHERE sling:vanityPath IS NOT NULL" +
+                Stream
+                    .of(true, false)
+                    .map(this::createVanityPathQueryPathRestriction)

Review comment:
       Good points! I will have another look at this. Also, a good point about 
moving the `/jcr:system`-exclude to the factory.

##########
File path: 
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
##########
@@ -1032,34 +1070,33 @@ private boolean addEntry(final Map<String, 
List<MapEntry>> entryMap, final Strin
     /*
     * Update alias query based on configured alias locations
     */
-    private String updateAliasQuery(){
+    private String createAliasQuery(){
         final Set<String> allowedLocations = 
this.factory.getAllowedAliasLocations();
 
-        StringBuilder baseQuery = new StringBuilder(ALIAS_BASE_QUERY_DEFAULT);
-        baseQuery.append(" ").append("WHERE");
+        StringBuilder query = new StringBuilder(ALIAS_BASE_QUERY_DEFAULT);
+        query.append(" ").append("WHERE");
 
         if(allowedLocations.isEmpty()){
-            String jcrSystemPath = StringUtils.removeEnd(JCR_SYSTEM_PREFIX, 
"/");
-            baseQuery.append(" ").append("(").append("NOT 
ISDESCENDANTNODE(page,")
-                    
.append("\"").append(jcrSystemPath).append("\"").append("))");
+            query.append(" ").append("(").append("NOT ISDESCENDANTNODE(page,")
+                    
.append("\"").append(JCR_SYSTEM_PATH).append("\"").append("))");

Review comment:
       Agree, I'll do that. I tried to touch as less as possible, but I very 
much agree with this.




-- 
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]


Reply via email to