You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by js...@apache.org on 2018/04/18 07:18:16 UTC

[sling-org-apache-sling-fsresource] branch master updated: SLING-7578 - FS Resource Provider has OS-dependent (unstable?) ordering of file-based resources

This is an automated email from the ASF dual-hosted git repository.

jsedding pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-fsresource.git


The following commit(s) were added to refs/heads/master by this push:
     new 2ab615f  SLING-7578 - FS Resource Provider has OS-dependent (unstable?) ordering of file-based resources
2ab615f is described below

commit 2ab615f42b98d33742d7354d4b7c2b066c92ccbb
Author: Julian Sedding <js...@apache.org>
AuthorDate: Wed Apr 18 09:17:37 2018 +0200

    SLING-7578 - FS Resource Provider has OS-dependent (unstable?) ordering of file-based resources
---
 .../internal/mapper/ContentFileResourceMapper.java |  2 ++
 .../internal/mapper/FileNameComparator.java        | 40 ++++++++++++++++++++++
 .../internal/mapper/FileResourceMapper.java        | 10 +++---
 .../internal/mapper/FileVaultResourceMapper.java   |  6 +++-
 .../fsprovider/internal/FileVaultContentTest.java  | 21 ++++--------
 .../sling/fsprovider/internal/FilesFolderTest.java |  4 +--
 .../fsprovider/internal/JcrXmlContentTest.java     |  2 +-
 .../sling/fsprovider/internal/JsonContentTest.java | 10 +++---
 .../content/samples/en/about_them/.content.xml     | 28 +++++++++++++++
 .../content/samples/en/about_us/.content.xml       | 28 +++++++++++++++
 10 files changed, 123 insertions(+), 28 deletions(-)

diff --git a/src/main/java/org/apache/sling/fsprovider/internal/mapper/ContentFileResourceMapper.java b/src/main/java/org/apache/sling/fsprovider/internal/mapper/ContentFileResourceMapper.java
index 509bd34..c69eb9e 100644
--- a/src/main/java/org/apache/sling/fsprovider/internal/mapper/ContentFileResourceMapper.java
+++ b/src/main/java/org/apache/sling/fsprovider/internal/mapper/ContentFileResourceMapper.java
@@ -20,6 +20,7 @@ package org.apache.sling.fsprovider.internal.mapper;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
@@ -99,6 +100,7 @@ public final class ContentFileResourceMapper implements FsResourceMapper {
         if (fileStatCache.isDirectory(parentDir)) {
             File[] files = parentDir.listFiles();
             if (files != null) {
+                Arrays.sort(files, FileNameComparator.INSTANCE);
                 childIterators.add(IteratorUtils.transformedIterator(IteratorUtils.arrayIterator(files), new Transformer() {
                     @Override
                     public Object transform(final Object input) {
diff --git a/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileNameComparator.java b/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileNameComparator.java
new file mode 100644
index 0000000..b5cb69e
--- /dev/null
+++ b/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileNameComparator.java
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.fsprovider.internal.mapper;
+
+import java.io.File;
+import java.util.Comparator;
+
+/**
+ * Comparator sorting files by filename. Intended to be used for
+ * comparing sibling files (i.e. files within the same folder).
+ */
+public class FileNameComparator implements java.util.Comparator<File> {
+
+    public static final Comparator<File> INSTANCE = new FileNameComparator();
+
+    private FileNameComparator() {
+        // singleton
+    }
+
+    @Override
+    public int compare(final File f1, final File f2) {
+        return f1.getName().compareTo(f2.getName());
+    }
+}
diff --git a/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileResourceMapper.java b/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileResourceMapper.java
index 61ed48c..690319d 100644
--- a/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileResourceMapper.java
+++ b/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileResourceMapper.java
@@ -19,6 +19,7 @@
 package org.apache.sling.fsprovider.internal.mapper;
 
 import java.io.File;
+import java.util.Arrays;
 import java.util.Iterator;
 
 import org.apache.commons.collections.IteratorUtils;
@@ -102,13 +103,14 @@ public final class FileResourceMapper implements FsResourceMapper {
                 return null;
             }
         }
-        
-        // ensure parent is a directory
-        if (!fileStatCache.isDirectory(parentFile)) {
+
+        File[] files = parentFile.listFiles();
+        if (files == null) {
             return null;
         }
 
-        Iterator<File> children = IteratorUtils.filteredIterator(IteratorUtils.arrayIterator(parentFile.listFiles()), new Predicate() {
+        Arrays.sort(files, FileNameComparator.INSTANCE);
+        Iterator<File> children = IteratorUtils.filteredIterator(IteratorUtils.arrayIterator(files), new Predicate() {
             @Override
             public boolean evaluate(Object object) {
                 File file = (File)object;
diff --git a/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileVaultResourceMapper.java b/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileVaultResourceMapper.java
index 90e66fd..39186ea 100644
--- a/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileVaultResourceMapper.java
+++ b/src/main/java/org/apache/sling/fsprovider/internal/mapper/FileVaultResourceMapper.java
@@ -23,6 +23,8 @@ import static org.apache.sling.fsprovider.internal.parser.ContentFileTypes.XML_S
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -114,7 +116,9 @@ public final class FileVaultResourceMapper implements FsResourceMapper {
         // additional check for children in file system
         File parentFile = getFile(parentPath);
         if (parentFile != null && fileStatCache.isDirectory(parentFile)) {
-            for (File childFile : parentFile.listFiles()) {
+            File[] files = parentFile.listFiles();
+            Arrays.sort(files, FileNameComparator.INSTANCE);
+            for (File childFile : files) {
                 String childPath = parentPath + "/" + PlatformNameFormat.getRepositoryName(childFile.getName());
                 File file = getFile(childPath);
                 if (file != null && pathMatches(childPath) && !childPaths.contains(childPath)) {
diff --git a/src/test/java/org/apache/sling/fsprovider/internal/FileVaultContentTest.java b/src/test/java/org/apache/sling/fsprovider/internal/FileVaultContentTest.java
index 16ff018..42a885d 100644
--- a/src/test/java/org/apache/sling/fsprovider/internal/FileVaultContentTest.java
+++ b/src/test/java/org/apache/sling/fsprovider/internal/FileVaultContentTest.java
@@ -111,20 +111,11 @@ public class FileVaultContentTest {
     @Test
     public void testListChildren() {
         Resource en = sampleContent.getChild("en");
-        List<Resource> children = ImmutableList.copyOf(en.listChildren());
-        assertEquals(3, children.size());
-        
-        Resource child1 = children.get(0);
-        assertEquals("jcr:content", child1.getName());
-        assertEquals("samples/sample-app/components/content/page/homepage", child1.getResourceType());
- 
-        Resource child2 = children.get(1);
-        assertEquals("tools", child2.getName());
-        assertEquals("app:Page", child2.getResourceType());
-        
-        Resource child3 = children.get(2);
-        assertEquals("extra", child3.getName());
-        
+        // about_us and about_them are not defined for ordering in en/.content.xml and thus sorted last
+        assertThat(en, ResourceMatchers.containsChildren("jcr:content", "tools", "extra", "about_them", "about_us"));
+        assertEquals("samples/sample-app/components/content/page/homepage", en.getChild("jcr:content").getResourceType());
+        assertEquals("app:Page", en.getChild("tools").getResourceType());
+
         // another child (conference) is hidden because of filter
     }
 
@@ -151,7 +142,7 @@ public class FileVaultContentTest {
 
         // list children with mixed content
         Resource enResource = sampleContent.getChild("en");
-        assertThat(enResource, ResourceMatchers.containsChildren("jcr:content", "tools", "extra", "conference"));
+        assertThat(enResource, ResourceMatchers.containsChildren("jcr:content", "tools", "extra", "about_them", "about_us", "conference"));
     }
 
     @Test
diff --git a/src/test/java/org/apache/sling/fsprovider/internal/FilesFolderTest.java b/src/test/java/org/apache/sling/fsprovider/internal/FilesFolderTest.java
index a704a77..3965583 100644
--- a/src/test/java/org/apache/sling/fsprovider/internal/FilesFolderTest.java
+++ b/src/test/java/org/apache/sling/fsprovider/internal/FilesFolderTest.java
@@ -76,8 +76,8 @@ public class FilesFolderTest {
     @Test
     public void testListChildren() {
         assertThat(root, ResourceMatchers.containsChildren("fs-test"));
-        assertThat(fsroot, ResourceMatchers.hasChildren("folder1", "folder2", "folder3"));
-        assertThat(fsroot.getChild("folder1"), ResourceMatchers.hasChildren("folder11", "file1a.txt", "sling:file1b.txt"));
+        assertThat(fsroot, ResourceMatchers.containsChildren("folder1", "folder2", "folder3"));
+        assertThat(fsroot.getChild("folder1"), ResourceMatchers.containsChildren("file1a.txt", "folder11", "sling:file1b.txt"));
         assertThat(fsroot.getChild("folder2"), ResourceMatchers.hasChildren("folder21", "content.json"));
         assertFalse(fsroot.getChild("folder1/file1a.txt").listChildren().hasNext());
     }
diff --git a/src/test/java/org/apache/sling/fsprovider/internal/JcrXmlContentTest.java b/src/test/java/org/apache/sling/fsprovider/internal/JcrXmlContentTest.java
index 3f51c1c..59c20d7 100644
--- a/src/test/java/org/apache/sling/fsprovider/internal/JcrXmlContentTest.java
+++ b/src/test/java/org/apache/sling/fsprovider/internal/JcrXmlContentTest.java
@@ -89,7 +89,7 @@ public class JcrXmlContentTest {
     public void testListChildren() {
         assertThat(root, ResourceMatchers.containsChildren("fs-test"));
         assertThat(fsroot, ResourceMatchers.hasChildren("folder1", "folder2"));
-        assertThat(fsroot.getChild("folder1"), ResourceMatchers.hasChildren("folder11", "file1a.txt", "sling:file1b.txt"));
+        assertThat(fsroot.getChild("folder1"), ResourceMatchers.containsChildren("file1a.txt", "folder11", "sling:file1b.txt"));
         assertThat(fsroot.getChild("folder2"), ResourceMatchers.hasChildren("folder21", "content"));
     }
 
diff --git a/src/test/java/org/apache/sling/fsprovider/internal/JsonContentTest.java b/src/test/java/org/apache/sling/fsprovider/internal/JsonContentTest.java
index 5c38ef1..21df79a 100644
--- a/src/test/java/org/apache/sling/fsprovider/internal/JsonContentTest.java
+++ b/src/test/java/org/apache/sling/fsprovider/internal/JsonContentTest.java
@@ -102,11 +102,11 @@ public class JsonContentTest {
     public void testListChildren() {
         assertThat(root, ResourceMatchers.containsChildren("fs-test"));
         assertThat(fsroot, ResourceMatchers.hasChildren("folder1", "folder2"));
-        assertThat(fsroot.getChild("folder1"), ResourceMatchers.containsChildrenInAnyOrder("folder11", "file1a.txt", "sling:file1b.txt"));
-        assertThat(fsroot.getChild("folder2"), ResourceMatchers.containsChildrenInAnyOrder("folder21", "content"));
-        assertThat(fsroot.getChild("folder2/content"), ResourceMatchers.containsChildrenInAnyOrder(
-                "jcr:content", "toolbar", "child", "file2content.txt", "sling:content2", "fileWithOverwrittenMimeType.scss"));
-        assertThat(fsroot.getChild("folder2/content/child"), ResourceMatchers.containsChildrenInAnyOrder("jcr:content", "grandchild"));
+        assertThat(fsroot.getChild("folder1"), ResourceMatchers.containsChildren("file1a.txt", "folder11", "sling:file1b.txt"));
+        assertThat(fsroot.getChild("folder2"), ResourceMatchers.containsChildren("content", "folder21"));
+        assertThat(fsroot.getChild("folder2/content"), ResourceMatchers.containsChildren(
+                "jcr:content", "toolbar", "child", "file2content.txt", "fileWithOverwrittenMimeType.scss", "sling:content2"));
+        assertThat(fsroot.getChild("folder2/content/child"), ResourceMatchers.containsChildren("jcr:content", "grandchild"));
     }
 
     @Test
diff --git a/src/test/resources/vaultfs-test/jcr_root/content/samples/en/about_them/.content.xml b/src/test/resources/vaultfs-test/jcr_root/content/samples/en/about_them/.content.xml
new file mode 100644
index 0000000..1f00e7f
--- /dev/null
+++ b/src/test/resources/vaultfs-test/jcr_root/content/samples/en/about_them/.content.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+    
+    http://www.apache.org/licenses/LICENSE-2.0
+    
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+-->
+<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:app="http://sample.com/jcr/app/1.0" xmlns:mix="http://www.jcp.org/jcr/mix/1.0" xmlns:sling="http://sling.apache.org/jcr/sling/1.0"
+    jcr:primaryType="app:Page">
+  <jcr:content
+      app:template="samples/sample-app/templates/admin/structureElement"
+      jcr:primaryType="app:PageContent"
+      jcr:title="about them"
+      sling:resourceType="samples/sample-app/components/admin/page/structureElement"/>
+  <navigation />
+</jcr:root>
diff --git a/src/test/resources/vaultfs-test/jcr_root/content/samples/en/about_us/.content.xml b/src/test/resources/vaultfs-test/jcr_root/content/samples/en/about_us/.content.xml
new file mode 100644
index 0000000..94432fc
--- /dev/null
+++ b/src/test/resources/vaultfs-test/jcr_root/content/samples/en/about_us/.content.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+    
+    http://www.apache.org/licenses/LICENSE-2.0
+    
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+-->
+<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:app="http://sample.com/jcr/app/1.0" xmlns:mix="http://www.jcp.org/jcr/mix/1.0" xmlns:sling="http://sling.apache.org/jcr/sling/1.0"
+    jcr:primaryType="app:Page">
+  <jcr:content
+      app:template="samples/sample-app/templates/admin/structureElement"
+      jcr:primaryType="app:PageContent"
+      jcr:title="about us"
+      sling:resourceType="samples/sample-app/components/admin/page/structureElement"/>
+  <navigation />
+</jcr:root>

-- 
To stop receiving notification emails like this one, please contact
jsedding@apache.org.