Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 2D524200CCF for ; Mon, 24 Jul 2017 16:27:39 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 2B86F1653DA; Mon, 24 Jul 2017 14:27:39 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 8E0641653D5 for ; Mon, 24 Jul 2017 16:27:37 +0200 (CEST) Received: (qmail 18662 invoked by uid 500); 24 Jul 2017 14:27:36 -0000 Mailing-List: contact commits-help@sling.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@sling.apache.org Delivered-To: mailing list commits@sling.apache.org Received: (qmail 18642 invoked by uid 99); 24 Jul 2017 14:27:36 -0000 Received: from Unknown (HELO svn01-us-west.apache.org) (209.188.14.144) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 24 Jul 2017 14:27:36 +0000 Received: from svn01-us-west.apache.org (localhost [127.0.0.1]) by svn01-us-west.apache.org (ASF Mail Server at svn01-us-west.apache.org) with ESMTP id 5C21C3A062F for ; Mon, 24 Jul 2017 14:27:34 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1802818 - in /sling/trunk/bundles/resourceresolver/src: main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java Date: Mon, 24 Jul 2017 14:27:32 -0000 To: commits@sling.apache.org From: pauls@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20170724142735.5C21C3A062F@svn01-us-west.apache.org> archived-at: Mon, 24 Jul 2017 14:27:39 -0000 Author: pauls Date: Mon Jul 24 14:27:32 2017 New Revision: 1802818 URL: http://svn.apache.org/viewvc?rev=1802818&view=rev Log: SLING-7018: Fix a bug that removed to many aliases in certain cases when a resource got removed. 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=1802818&r1=1802817&r2=1802818&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 Mon Jul 24 14:27:32 2017 @@ -331,7 +331,7 @@ public class MapEntries implements if (this.factory.isOptimizeAliasResolutionEnabled()) { for (final String contentPath : this.aliasMap.keySet()) { if (path.startsWith(contentPath + "/") || path.equals(contentPath)) { - changed |= removeAlias(contentPath, null, resolverRefreshed); + changed |= removeAlias(contentPath, path, resolverRefreshed); } else if ( contentPath.startsWith(actualContentPathPrefix) ) { changed |= removeAlias(contentPath, path, resolverRefreshed); } @@ -352,37 +352,59 @@ public class MapEntries implements // a direct child of vanity path but not jcr:content, or a jcr:content child of a direct child // otherwise we can discard the event boolean handle = true; - String addParentPath = null; - if ( path != null ) { + String resourcePath = null; + if ( path != null && path.length() > contentPath.length()) { final String subPath = path.substring(contentPath.length() + 1); final int firstSlash = subPath.indexOf('/'); if ( firstSlash == -1 ) { if ( subPath.equals(JCR_CONTENT) ) { handle = false; } + resourcePath = path; } else if ( subPath.lastIndexOf('/') == firstSlash) { if ( subPath.startsWith(JCR_CONTENT_PREFIX) || !subPath.endsWith(JCR_CONTENT_SUFFIX) ) { handle = false; } - addParentPath = ResourceUtil.getParent(path); + resourcePath = ResourceUtil.getParent(path); } else { handle = false; } } + else { + resourcePath = contentPath; + } if ( !handle ) { return false; } this.initializing.lock(); try { - final Map aliasMapEntry = aliasMap.remove(contentPath); - if (aliasMapEntry != null && addParentPath != null ) { + final Map aliasMapEntry = aliasMap.get(contentPath); + if (aliasMapEntry != null) { this.refreshResolverIfNecessary(resolverRefreshed); - // we need to re-add - // from a potential parent - final Resource parent = this.resolver.getResource(addParentPath); - if ( parent != null && parent.getValueMap().containsKey(ResourceResolverImpl.PROP_ALIAS)) { - doAddAlias(parent); + + for (Iterator> iterator = aliasMapEntry.entrySet().iterator(); iterator.hasNext(); ) { + final Map.Entry entry = iterator.next(); + String prefix = contentPath.endsWith("/") ? contentPath : contentPath + "/"; + if ((prefix + entry.getValue()).startsWith(resourcePath)){ + iterator.remove(); + } + } + + if (aliasMapEntry.isEmpty()) { + this.aliasMap.remove(contentPath); + } + + Resource containingResource = this.resolver.getResource(resourcePath); + + if (containingResource != null) { + if (containingResource.getValueMap().containsKey(ResourceResolverImpl.PROP_ALIAS)) { + doAddAlias(containingResource); + } + final Resource child = containingResource.getChild(JCR_CONTENT); + if (child != null && child.getValueMap().containsKey(ResourceResolverImpl.PROP_ALIAS)) { + doAddAlias(child); + } } } return aliasMapEntry != null; 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=1802818&r1=1802817&r2=1802818&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 Mon Jul 24 14:27:32 2017 @@ -1183,6 +1183,7 @@ public class MapEntriesTest { assertEquals(1, aliasMap.size()); + when(resourceResolver.getResource("/parent/child")).thenReturn(null); removeAlias.invoke(mapEntries, "/parent", "/parent/child", new AtomicBoolean()); aliasMapEntry = mapEntries.getAliasMap("/parent"); @@ -1191,6 +1192,7 @@ public class MapEntriesTest { assertEquals(0, aliasMap.size()); //re-add node and test nodeDeletion true + when(resourceResolver.getResource("/parent/child")).thenReturn(child); addResource.invoke(mapEntries, "/parent/child", new AtomicBoolean()); aliasMapEntry = mapEntries.getAliasMap("/parent"); @@ -1247,6 +1249,8 @@ public class MapEntriesTest { assertEquals(1, aliasMap.size()); + when(resourceResolver.getResource("/parent/child/jcr:content")).thenReturn(null); + when(result.getChild("jcr:content")).thenReturn(null); removeAlias.invoke(mapEntries, "/parent", "/parent/child/jcr:content", new AtomicBoolean()); aliasMapEntry = mapEntries.getAliasMap("/parent"); @@ -1255,6 +1259,8 @@ public class MapEntriesTest { assertEquals(0, aliasMap.size()); //re-add node and test nodeDeletion true + when(resourceResolver.getResource("/parent/child/jcr:content")).thenReturn(jcrContentResult); + when(result.getChild("jcr:content")).thenReturn(jcrContentResult); addResource.invoke(mapEntries, "/parent/child/jcr:content", new AtomicBoolean()); aliasMapEntry = mapEntries.getAliasMap("/parent"); @@ -1314,6 +1320,8 @@ public class MapEntriesTest { assertEquals("child", aliasMapEntry.get("alias")); // remove child jcr:content node + when(resourceResolver.getResource("/parent/child/jcr:content")).thenReturn(null); + when(childRsrc.getChild("jcr:content")).thenReturn(null); removeAlias.invoke(mapEntries, "/parent", "/parent/child/jcr:content", new AtomicBoolean()); // test with one node @@ -1324,6 +1332,8 @@ public class MapEntriesTest { assertEquals("child", aliasMapEntry.get("alias")); // re-add the node and test /parent/child + when(resourceResolver.getResource("/parent/child/jcr:content")).thenReturn(jcrContentResult); + when(childRsrc.getChild("jcr:content")).thenReturn(jcrContentResult); addResource.invoke(mapEntries, "/parent/child/jcr:content", new AtomicBoolean()); // STOP @@ -1333,7 +1343,9 @@ public class MapEntriesTest { assertEquals("child", aliasMapEntry.get("aliasJcrContent")); assertEquals("child", aliasMapEntry.get("alias")); + when(resourceResolver.getResource("/parent/child")).thenReturn(null); removeAlias.invoke(mapEntries, "/parent", "/parent/child", new AtomicBoolean()); + when(resourceResolver.getResource("/parent/child")).thenReturn(childRsrc); addResource.invoke(mapEntries, "/parent/child/jcr:content", new AtomicBoolean()); assertEquals(1, aliasMap.size()); @@ -1372,6 +1384,7 @@ public class MapEntriesTest { assertEquals("child", aliasMapEntry.get("aliasJcrContent")); assertEquals(2, aliasMapEntry.size()); + when(resourceResolver.getResource("/parent/child")).thenReturn( null); removeAlias.invoke(mapEntries, "/parent", "/parent/child", new AtomicBoolean()); assertEquals(0, aliasMap.size()); @@ -1408,6 +1421,7 @@ public class MapEntriesTest { assertEquals(1, aliasMap.size()); + when(resourceResolver.getResource("/parent")).thenReturn(null); removeAlias.invoke(mapEntries, "/", "/parent", new AtomicBoolean()); aliasMapEntry = mapEntries.getAliasMap("/"); @@ -1416,6 +1430,7 @@ public class MapEntriesTest { assertEquals(0, aliasMap.size()); //re-add node and test nodeDeletion true + when(resourceResolver.getResource("/parent")).thenReturn(result); addResource.invoke(mapEntries, "/parent", new AtomicBoolean()); aliasMapEntry = mapEntries.getAliasMap("/"); @@ -1472,30 +1487,152 @@ public class MapEntriesTest { assertEquals(1, aliasMap.size()); + when(resourceResolver.getResource("/parent/jcr:content")).thenReturn(null); + when(result.getChild("jcr:content")).thenReturn(null); removeAlias.invoke(mapEntries, "/", "/parent/jcr:content", new AtomicBoolean()); aliasMapEntry = mapEntries.getAliasMap("/"); assertNull(aliasMapEntry); assertEquals(0, aliasMap.size()); + } - //re-add node and test nodeDeletion true - addResource.invoke(mapEntries, "/parent/jcr:content", new AtomicBoolean()); + @Test + public void test_doRemoveAliasFromSibling() throws Exception { + final Method addResource = MapEntries.class.getDeclaredMethod("addResource", String.class, AtomicBoolean.class); + addResource.setAccessible(true); - aliasMapEntry = mapEntries.getAliasMap("/"); + final Method removeResource = MapEntries.class.getDeclaredMethod("removeResource", String.class, AtomicBoolean.class); + removeResource.setAccessible(true); + + assertEquals(0, aliasMap.size()); + + Resource parent = mock(Resource.class); + when(parent.getPath()).thenReturn("/parent"); + + when(resourceResolver.getResource("/parent")).thenReturn(parent); + when(parent.getParent()).thenReturn(parent); + when(parent.getPath()).thenReturn("/parent"); + when(parent.getName()).thenReturn("parent"); + when(parent.getValueMap()).thenReturn(buildValueMap()); + + final Resource child1 = mock(Resource.class); + when(resourceResolver.getResource("/parent/child1")).thenReturn(child1); + when(child1.getParent()).thenReturn(parent); + when(child1.getPath()).thenReturn("/parent/child1"); + when(child1.getName()).thenReturn("child1"); + when(child1.getValueMap()).thenReturn(buildValueMap()); + + final Resource child1JcrContent = mock(Resource.class); + when(resourceResolver.getResource("/parent/child1/jcr:content")).thenReturn(child1JcrContent); + when(child1JcrContent.getParent()).thenReturn(child1); + when(child1JcrContent.getPath()).thenReturn("/parent/child1/jcr:content"); + when(child1JcrContent.getName()).thenReturn("jcr:content"); + when(child1JcrContent.getValueMap()).thenReturn(buildValueMap(ResourceResolverImpl.PROP_ALIAS, "test1")); + when(child1.getChild("jcr:content")).thenReturn(child1JcrContent); + + when(parent.getChild("child1")).thenReturn(child1); + + addResource.invoke(mapEntries, child1JcrContent.getPath(), new AtomicBoolean()); + + Map aliasMapEntry = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMapEntry); - assertTrue(aliasMapEntry.containsKey("aliasJcrContent")); - assertEquals("parent", aliasMapEntry.get("aliasJcrContent")); + assertTrue(aliasMapEntry.containsKey("test1")); + assertEquals("child1", aliasMapEntry.get("test1")); assertEquals(1, aliasMap.size()); - when(resourceResolver.getResource("/parent/jcr:content")).thenReturn(null); - when(result.getChild("jcr:content")).thenReturn(null); - removeAlias.invoke(mapEntries, "/", "/parent/jcr:content", new AtomicBoolean()); - aliasMapEntry = mapEntries.getAliasMap("/"); + final Resource child2 = mock(Resource.class); + when(resourceResolver.getResource("/parent/child2")).thenReturn(child2); + when(child2.getParent()).thenReturn(parent); + when(child2.getPath()).thenReturn("/parent/child2"); + when(child2.getName()).thenReturn("child2"); + when(child2.getValueMap()).thenReturn(buildValueMap()); + + final Resource child2JcrContent = mock(Resource.class); + when(resourceResolver.getResource("/parent/child2/jcr:content")).thenReturn(child2JcrContent); + when(child2JcrContent.getParent()).thenReturn(child2); + when(child2JcrContent.getPath()).thenReturn("/parent/child2/jcr:content"); + when(child2JcrContent.getName()).thenReturn("jcr:content"); + when(child2JcrContent.getValueMap()).thenReturn(buildValueMap(ResourceResolverImpl.PROP_ALIAS, "test2")); + when(child2.getChild("jcr:content")).thenReturn(child2JcrContent); + + when(parent.getChild("child2")).thenReturn(child2); + + addResource.invoke(mapEntries, child2JcrContent.getPath(), new AtomicBoolean()); + + aliasMapEntry = mapEntries.getAliasMap("/parent"); + assertNotNull(aliasMapEntry); + assertTrue(aliasMapEntry.containsKey("test1")); + assertTrue(aliasMapEntry.containsKey("test2")); + assertEquals("child1", aliasMapEntry.get("test1")); + assertEquals("child2", aliasMapEntry.get("test2")); + + + assertEquals(1, aliasMap.size()); + assertEquals(2, mapEntries.getAliasMap("/parent").size()); + + final Resource child2JcrContentChild = mock(Resource.class); + when(resourceResolver.getResource("/parent/child2/jcr:content/test")).thenReturn(child2JcrContentChild); + when(child2JcrContentChild.getParent()).thenReturn(child2); + when(child2JcrContentChild.getPath()).thenReturn("/parent/child2/jcr:content/test"); + when(child2JcrContentChild.getName()).thenReturn("test"); + when(child2JcrContent.getChild("test")).thenReturn(child2JcrContentChild); + + removeResource.invoke(mapEntries, child2JcrContentChild.getPath(), new AtomicBoolean()); + + aliasMapEntry = mapEntries.getAliasMap("/parent"); + assertNotNull(aliasMapEntry); + assertTrue(aliasMapEntry.containsKey("test1")); + assertTrue(aliasMapEntry.containsKey("test2")); + assertEquals("child1", aliasMapEntry.get("test1")); + assertEquals("child2", aliasMapEntry.get("test2")); + + + assertEquals(1, aliasMap.size()); + assertEquals(2, mapEntries.getAliasMap("/parent").size()); + + when(child2.getChild("jcr:content")).thenReturn(null); + + removeResource.invoke(mapEntries, child2JcrContent.getPath(), new AtomicBoolean()); + + aliasMapEntry = mapEntries.getAliasMap("/parent"); + assertNotNull(aliasMapEntry); + assertTrue(aliasMapEntry.containsKey("test1")); + assertEquals("child1", aliasMapEntry.get("test1")); + + + assertEquals(1, aliasMap.size()); + assertEquals(1, mapEntries.getAliasMap("/parent").size()); + + when(child1.getChild("jcr:content")).thenReturn(null); + + removeResource.invoke(mapEntries, child1JcrContent.getPath(), new AtomicBoolean()); + + aliasMapEntry = mapEntries.getAliasMap("/parent"); assertNull(aliasMapEntry); - assertEquals(0, aliasMap.size()); + when(child1.getChild("jcr:content")).thenReturn(child1JcrContent); + addResource.invoke(mapEntries, child1JcrContent.getPath(), new AtomicBoolean()); + when(child2.getChild("jcr:content")).thenReturn(child2JcrContent); + addResource.invoke(mapEntries, child2JcrContent.getPath(), new AtomicBoolean()); + + aliasMapEntry = mapEntries.getAliasMap("/parent"); + assertNotNull(aliasMapEntry); + assertTrue(aliasMapEntry.containsKey("test1")); + assertTrue(aliasMapEntry.containsKey("test2")); + assertEquals("child1", aliasMapEntry.get("test1")); + assertEquals("child2", aliasMapEntry.get("test2")); + + + assertEquals(1, aliasMap.size()); + assertEquals(2, mapEntries.getAliasMap("/parent").size()); + + removeResource.invoke(mapEntries, parent.getPath(), new AtomicBoolean()); + + + aliasMapEntry = mapEntries.getAliasMap("/parent"); + assertNull(aliasMapEntry); } @Test