Author: cziegeler Date: Fri Mar 24 15:59:58 2017 New Revision: 1788490 URL: http://svn.apache.org/viewvc?rev=1788490&view=rev Log: SLING-6710 : Vanity Path might get removed if a resource is updated
Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java?rev=1788490&r1=1788489&r2=1788490&view=diff ============================================================================== --- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java (original) +++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java Fri Mar 24 15:59:58 2017 @@ -61,6 +61,7 @@ import org.apache.sling.api.resource.obs import org.apache.sling.api.resource.observation.ResourceChange; import org.apache.sling.api.resource.observation.ResourceChangeListener; import org.apache.sling.api.resource.path.Path; +import org.apache.sling.resourceresolver.impl.ResourceResolverFactoryImpl; import org.apache.sling.resourceresolver.impl.ResourceResolverImpl; import org.apache.sling.resourceresolver.impl.mapping.MapConfigurationProvider.VanityPathConfig; import org.osgi.framework.BundleContext; @@ -155,7 +156,7 @@ public class MapEntries implements doInit(); - final Dictionary<String, Object> props = new Hashtable<String, Object>(); + final Dictionary<String, Object> props = new Hashtable<>(); final String[] paths = new String[factory.getObservationPaths().length]; for(int i=0 ; i < paths.length; i++) { paths[i] = factory.getObservationPaths()[i].getPath(); @@ -186,7 +187,7 @@ public class MapEntries implements return; } - final Map<String, List<MapEntry>> newResolveMapsMap = new ConcurrentHashMap<String, List<MapEntry>>(); + final Map<String, List<MapEntry>> newResolveMapsMap = new ConcurrentHashMap<>(); //optimization made in SLING-2521 if (this.factory.isOptimizeAliasResolutionEnabled()) { @@ -268,12 +269,8 @@ public class MapEntries implements this.refreshResolverIfNecessary(resolverRefreshed); final Resource resource = resolver.getResource(path); if (resource != null) { - boolean changed = false; - final ValueMap props = resource.getValueMap(); - if (props.containsKey(PROP_VANITY_PATH)) { - changed |= doAddVanity(resource, props); - } - if (this.factory.isOptimizeAliasResolutionEnabled() && props.containsKey(ResourceResolverImpl.PROP_ALIAS)) { + boolean changed = doAddVanity(resource); + if (this.factory.isOptimizeAliasResolutionEnabled() && resource.getValueMap().containsKey(ResourceResolverImpl.PROP_ALIAS)) { changed |= doAddAlias(resource); } return changed; @@ -286,31 +283,39 @@ public class MapEntries implements } private boolean updateResource(final String path, final AtomicBoolean resolverRefreshed) { - this.initializing.lock(); - - try { - this.refreshResolverIfNecessary(resolverRefreshed); - final Resource resource = resolver.getResource(path); - if (resource != null) { - boolean changed = false; - final ValueMap props = resource.getValueMap(); + final boolean isValidVanityPath = this.isValidVanityPath(path); + if ( this.factory.isOptimizeAliasResolutionEnabled() || isValidVanityPath) { + this.initializing.lock(); - changed |= doRemoveVanity(path); - if (props.containsKey(PROP_VANITY_PATH)) { - changed |= doAddVanity(resource, props); - } + try { + this.refreshResolverIfNecessary(resolverRefreshed); + final Resource resource = resolver.getResource(path); + if (resource != null) { + boolean changed = false; + if ( isValidVanityPath ) { + // we remove the old vanity path first + changed |= doRemoveVanity(path); + + // add back vanity path + Resource contentRsrc = null; + if ( !resource.getName().equals(JCR_CONTENT)) { + // there might be a JCR_CONTENT child resource + contentRsrc = resource.getChild(JCR_CONTENT); + } + changed |= doAddVanity(contentRsrc != null ? contentRsrc : resource); + } + if (this.factory.isOptimizeAliasResolutionEnabled()) { + changed |= doUpdateAlias(resource); + } - if (this.factory.isOptimizeAliasResolutionEnabled()) { - changed |= doUpdateAlias(resource); + return changed; } - - return changed; + } finally { + this.initializing.unlock(); } - - return false; - } finally { - this.initializing.unlock(); } + + return false; } private boolean removeResource(final String path, final AtomicBoolean resolverRefreshed) { @@ -400,8 +405,8 @@ public class MapEntries implements * Does no locking and does not send an event at the end */ private void doUpdateConfiguration() { - final List<MapEntry> globalResolveMap = new ArrayList<MapEntry>(); - final SortedMap<String, MapEntry> newMapMaps = new TreeMap<String, MapEntry>(); + final List<MapEntry> globalResolveMap = new ArrayList<>(); + final SortedMap<String, MapEntry> newMapMaps = new TreeMap<>(); // load the /etc/map entries into the maps loadResolverMap(resolver, globalResolveMap, newMapMaps); // load the configuration into the resolver map @@ -411,10 +416,10 @@ public class MapEntries implements // sort global list and add to map Collections.sort(globalResolveMap); resolveMapsMap.put(GLOBAL_LIST_KEY, globalResolveMap); - this.mapMaps = Collections.unmodifiableSet(new TreeSet<MapEntry>(newMapMaps.values())); + this.mapMaps = Collections.unmodifiableSet(new TreeSet<>(newMapMaps.values())); } - private boolean doAddVanity(final Resource resource, final ValueMap props) { + private boolean doAddVanity(final Resource resource) { log.debug("doAddVanity getting {}", resource.getPath()); boolean needsUpdate = false; @@ -573,7 +578,7 @@ public class MapEntries implements */ @Override public List<MapEntry> getResolveMaps() { - final List<MapEntry> entries = new ArrayList<MapEntry>(); + final List<MapEntry> entries = new ArrayList<>(); for (final List<MapEntry> list : this.resolveMapsMap.values()) { entries.addAll(list); } @@ -793,7 +798,7 @@ public class MapEntries implements */ private Map<String, List<MapEntry>> getVanityPaths(String vanityPath) { - Map<String, List<MapEntry>> entryMap = new HashMap<String, List<MapEntry>>(); + Map<String, List<MapEntry>> entryMap = new HashMap<>(); // sling:vanityPath (lowercase) is the property name final String queryString = "SELECT sling:vanityPath, sling:redirect, sling:redirectStatus FROM nt:base WHERE sling:vanityPath =" @@ -818,7 +823,7 @@ public class MapEntries implements loadVanityPath(resource, resolveMapsMap, vanityTargets, true, false); entryMap = resolveMapsMap; } else { - final Map <String, List<String>> targetPaths = new HashMap <String, List<String>>(); + final Map <String, List<String>> targetPaths = new HashMap <>(); loadVanityPath(resource, entryMap, targetPaths, true, false); } } @@ -834,18 +839,18 @@ public class MapEntries implements } /** - * Check if the resource is a valid vanity path resource - * @param resource The resource to check + * Check if the path is a valid vanity path + * @param path The resource path to check * @return {@code true} if this is valid, {@code false} otherwise */ - private boolean isValidVanityPath(Resource resource){ - if(resource == null) { - throw new IllegalArgumentException("Unexpected null resource"); + private boolean isValidVanityPath(final String path){ + if (path == null) { + throw new IllegalArgumentException("Unexpected null path"); } // ignore system tree - if (resource.getPath().startsWith(JCR_SYSTEM_PREFIX)) { - log.debug("isValidVanityPath: not valid {}", resource); + if (path.startsWith(JCR_SYSTEM_PREFIX)) { + log.debug("isValidVanityPath: not valid {}", path); return false; } @@ -853,13 +858,13 @@ public class MapEntries implements if ( this.factory.getVanityPathConfig() != null ) { boolean allowed = false; for(final VanityPathConfig config : this.factory.getVanityPathConfig()) { - if ( resource.getPath().startsWith(config.prefix) ) { + if ( path.startsWith(config.prefix) ) { allowed = !config.isExclude; break; } } if ( !allowed ) { - log.debug("isValidVanityPath: not valid as not in white list {}", resource); + log.debug("isValidVanityPath: not valid as not in white list {}", path); return false; } } @@ -977,13 +982,13 @@ public class MapEntries implements List<MapEntry> entries = entryMap.get(key); if (entries == null) { - entries = new ArrayList<MapEntry>(); + entries = new ArrayList<>(); entries.add(entry); // and finally sort list Collections.sort(entries); entryMap.put(key, entries); } else { - List<MapEntry> entriesCopy =new ArrayList<MapEntry>(entries); + List<MapEntry> entriesCopy =new ArrayList<>(entries); entriesCopy.add(entry); // and finally sort list Collections.sort( entriesCopy); @@ -997,7 +1002,7 @@ public class MapEntries implements * property */ private Map<String, Map<String, String>> loadAliases(final ResourceResolver resolver) { - final Map<String, Map<String, String>> map = new ConcurrentHashMap<String, Map<String, String>>(); + final Map<String, Map<String, String>> map = new ConcurrentHashMap<>(); final String queryString = "SELECT sling:alias FROM nt:base WHERE sling:alias IS NOT NULL"; final Iterator<Resource> i = resolver.findResources(queryString, "sql"); while (i.hasNext()) { @@ -1077,7 +1082,7 @@ public class MapEntries implements alias, parentPath); } else { if (parentMap == null) { - parentMap = new LinkedHashMap<String, String>(); + parentMap = new LinkedHashMap<>(); map.put(parentPath, parentMap); } parentMap.put(alias, resourceName); @@ -1096,7 +1101,7 @@ public class MapEntries implements */ private Map <String, List<String>> loadVanityPaths(boolean createVanityBloomFilter) { // sling:vanityPath (lowercase) is the property name - final Map <String, List<String>> targetPaths = new ConcurrentHashMap <String, List<String>>(); + final Map <String, List<String>> targetPaths = new ConcurrentHashMap <>(); final String queryString = "SELECT sling:vanityPath, sling:redirect, sling:redirectStatus FROM nt:base WHERE sling:vanityPath IS NOT NULL"; final Iterator<Resource> i = resolver.findResources(queryString, "sql"); @@ -1132,7 +1137,7 @@ public class MapEntries implements */ private boolean loadVanityPath(final Resource resource, final Map<String, List<MapEntry>> entryMap, final Map <String, List<String>> targetPaths, boolean addToCache, boolean newVanity) { - if (!isValidVanityPath(resource)) { + if (!isValidVanityPath(resource.getPath())) { return false; } @@ -1221,7 +1226,7 @@ public class MapEntries implements } List<String> entries = targetPaths.get(key); if (entries == null) { - entries = new ArrayList<String>(); + entries = new ArrayList<>(); targetPaths.put(key, entries); } entries.add(entry); @@ -1293,7 +1298,7 @@ public class MapEntries implements // URL Mappings final Mapping[] mappings = factory.getMappings(); if (mappings != null) { - final Map<String, List<String>> map = new HashMap<String, List<String>>(); + final Map<String, List<String>> map = new HashMap<>(); for (final Mapping mapping : mappings) { if (mapping.mapsInbound()) { final String url = mapping.getTo(); @@ -1301,7 +1306,7 @@ public class MapEntries implements if (url.length() > 0) { List<String> aliasList = map.get(url); if (aliasList == null) { - aliasList = new ArrayList<String>(); + aliasList = new ArrayList<>(); map.put(url, aliasList); } aliasList.add(alias); Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java?rev=1788490&r1=1788489&r2=1788490&view=diff ============================================================================== --- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java (original) +++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java Fri Mar 24 15:59:58 2017 @@ -101,7 +101,7 @@ public class MapEntriesTest { public void setup() throws Exception { MockitoAnnotations.initMocks(this); - final List<VanityPathConfig> configs = new ArrayList<MapConfigurationProvider.VanityPathConfig>(); + final List<VanityPathConfig> configs = new ArrayList<>(); configs.add(new VanityPathConfig("/libs/", false)); configs.add(new VanityPathConfig("/libs/denied", true)); configs.add(new VanityPathConfig("/foo/", false)); @@ -218,7 +218,7 @@ public class MapEntriesTest { when(resourceResolverFactory.getDefaultVanityPathRedirectStatus()).thenReturn(DEFAULT_VANITY_STATUS); - final List<Resource> resources = new ArrayList<Resource>(); + final List<Resource> resources = new ArrayList<>(); Resource justVanityPath = mock(Resource.class, "justVanityPath"); when(justVanityPath.getPath()).thenReturn("/justVanityPath"); @@ -298,7 +298,7 @@ public class MapEntriesTest { } private ValueMap buildValueMap(Object... string) { - final Map<String, Object> data = new HashMap<String, Object>(); + final Map<String, Object> data = new HashMap<>(); for (int i = 0; i < string.length; i = i + 2) { data.put((String) string[i], string[i+1]); } @@ -318,7 +318,7 @@ public class MapEntriesTest { final String[] validPaths = {"/libs/somewhere", "/libs/a/b", "/foo/a", "/baa/a"}; final String[] invalidPaths = {"/libs/denied/a", "/libs/denied/b/c", "/nowhere"}; - final List<Resource> resources = new ArrayList<Resource>(); + final List<Resource> resources = new ArrayList<>(); for(final String val : validPaths) { resources.add(getVanityPathResource(val)); } @@ -346,7 +346,7 @@ public class MapEntriesTest { // each valid resource results in 2 entries assertEquals(validPaths.length * 2, entries.size()); - final Set<String> resultSet = new HashSet<String>(); + final Set<String> resultSet = new HashSet<>(); for(final String p : validPaths) { resultSet.add(p + "$1"); resultSet.add(p + ".html"); @@ -1426,18 +1426,12 @@ public class MapEntriesTest { @Test public void test_isValidVanityPath() throws Exception { - Method method = MapEntries.class.getDeclaredMethod("isValidVanityPath", Resource.class); + Method method = MapEntries.class.getDeclaredMethod("isValidVanityPath", String.class); method.setAccessible(true); - final Resource resource = mock(Resource.class); - when(resource.getPath()).thenReturn("/jcr:system/node"); + assertFalse((Boolean)method.invoke(mapEntries, "/jcr:system/node")); - assertFalse((Boolean)method.invoke(mapEntries, resource)); - - when(resource.getPath()).thenReturn("/justVanityPath"); - when(resource.getValueMap()).thenReturn(mock(ValueMap.class)); - - assertTrue((Boolean)method.invoke(mapEntries, resource)); + assertTrue((Boolean)method.invoke(mapEntries, "/justVanityPath")); } @Test @@ -1803,7 +1797,7 @@ public class MapEntriesTest { when(this.resourceResolverFactory.getMaxCachedVanityPathEntries()).thenReturn(2L); - ArrayList<DataFuture> list = new ArrayList<DataFuture>(); + ArrayList<DataFuture> list = new ArrayList<>(); for (int i =0;i<10;i++) { list.add(createDataFuture(pool, mapEntries)); @@ -1814,7 +1808,7 @@ public class MapEntriesTest { } } - + // tests SLING-6542 @Test public void sessionConcurrency() throws Exception { @@ -1822,7 +1816,7 @@ public class MapEntriesTest { addResource.setAccessible(true); final Method updateResource = MapEntries.class.getDeclaredMethod("updateResource", String.class, AtomicBoolean.class); updateResource.setAccessible(true); - final Method handleConfigurationUpdate = MapEntries.class.getDeclaredMethod("handleConfigurationUpdate", + final Method handleConfigurationUpdate = MapEntries.class.getDeclaredMethod("handleConfigurationUpdate", String.class, AtomicBoolean.class, AtomicBoolean.class, boolean.class); handleConfigurationUpdate.setAccessible(true); @@ -1837,7 +1831,7 @@ public class MapEntriesTest { simulateSomewhatSlowSessionOperation(sessionLock); return null; } - + }).when(resourceResolver).refresh(); Mockito.doAnswer(new Answer<Resource>() { @@ -1846,13 +1840,13 @@ public class MapEntriesTest { simulateSomewhatSlowSessionOperation(sessionLock); return null; } - + }).when(resourceResolver).getResource(any(String.class)); - + when(resourceResolverFactory.isMapConfiguration(any(String.class))).thenReturn(true); - + final AtomicInteger failureCnt = new AtomicInteger(0); - final List<Exception> exceptions = new LinkedList<Exception>(); + final List<Exception> exceptions = new LinkedList<>(); final Semaphore done = new Semaphore(0); final int NUM_THREADS = 30; final Random random = new Random(12321); @@ -1925,5 +1919,5 @@ public class MapEntriesTest { this.future = future; } } - + } \ No newline at end of file