fix #10822: add "local" options to omero.fs.repo.path_rules

components/blitz/src/ome/services/blitz/repo/path/FilePathRestrictionInstance.java

@@ -22,6 +22,8 @@
 import java.util.HashSet;
 import java.util.Set;
 
+import ome.services.blitz.util.CurrentPlatform;
+
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -41,13 +43,13 @@ public enum FilePathRestrictionInstance {
     /** proscribe naming that probably causes system problems on UNIX-like platforms */
     UNIX_REQUIRED("UNIX required"),
     /** proscribe naming that probably causes sysadmin inconvenience on UNIX-like platforms */
-    UNIX_OPTIONAL("UNIX optional");
+    UNIX_OPTIONAL("UNIX optional"),
+    /** proscribe naming that probably causes system problems on the current platform */
+    LOCAL_REQUIRED("local required"),
+    /** proscribe naming that probably causes sysadmin inconvenience on the current platform */
+    LOCAL_OPTIONAL("local optional");
 
-    private static ImmutableMap<String, FilePathRestrictionInstance> nameLookup =
-            ImmutableMap.of(WINDOWS_REQUIRED.name, WINDOWS_REQUIRED,
-                            WINDOWS_OPTIONAL.name, WINDOWS_OPTIONAL,
-                            UNIX_REQUIRED.name, UNIX_REQUIRED,
-                            UNIX_OPTIONAL.name, UNIX_OPTIONAL);
+    private static ImmutableMap<String, FilePathRestrictionInstance> nameLookup;
 
     /**
      * Convert the given character to the corresponding Unicode code point.
@@ -63,6 +65,15 @@ static int getCodePoint(char character) {
         /* Some information on Windows naming strategies is to be found at
          * http://msdn.microsoft.com/en-us/library/aa365247(VS.85).aspx */
 
+        final ImmutableMap.Builder<String, FilePathRestrictionInstance> nameLookupBuilder = ImmutableMap.builder();
+        nameLookupBuilder.put(WINDOWS_REQUIRED.name, WINDOWS_REQUIRED);
+        nameLookupBuilder.put(WINDOWS_OPTIONAL.name, WINDOWS_OPTIONAL);
+        nameLookupBuilder.put(UNIX_REQUIRED.name, UNIX_REQUIRED);
+        nameLookupBuilder.put(UNIX_OPTIONAL.name, UNIX_OPTIONAL);
+        nameLookupBuilder.put(LOCAL_REQUIRED.name, LOCAL_REQUIRED);
+        nameLookupBuilder.put(LOCAL_OPTIONAL.name, LOCAL_OPTIONAL);
+        nameLookup = nameLookupBuilder.build();
+
         final ImmutableSet<Character> safeCharacters = ImmutableSet.of('_');
         final int safeCodePoint = getCodePoint(safeCharacters.iterator().next());
 
@@ -119,6 +130,18 @@ static int getCodePoint(char character) {
         unsafePrefixes.add("-");
 
         UNIX_OPTIONAL.rules = new FilePathRestrictions(null, unsafePrefixes, null, null, safeCharacters);
+
+        if (CurrentPlatform.isWindows()) {
+            LOCAL_REQUIRED.rules = WINDOWS_REQUIRED.rules;
+            LOCAL_OPTIONAL.rules = WINDOWS_OPTIONAL.rules;
+        } else if (CurrentPlatform.isLinux() || CurrentPlatform.isMacOSX()) {
+            LOCAL_REQUIRED.rules = UNIX_REQUIRED.rules;
+            LOCAL_OPTIONAL.rules = UNIX_OPTIONAL.rules;
+        } else {
+            /* take a conservative approach */
+            LOCAL_REQUIRED.rules = getFilePathRestrictions(WINDOWS_REQUIRED, UNIX_REQUIRED);
+            LOCAL_OPTIONAL.rules = getFilePathRestrictions(WINDOWS_OPTIONAL, UNIX_OPTIONAL);
+        }
     }
 
     public final String name;

components/blitz/test/ome/services/blitz/test/utests/PlatformAssumptions.java → components/blitz/src/ome/services/blitz/util/CurrentPlatform.java

@@ -17,19 +17,23 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-package ome.services.blitz.test.utests;
+package ome.services.blitz.util;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * Methods for use with {@literal @}Assumption annotations
- * to limit unit tests to specific test platforms.
- * 
+ * Test under which platform OMERO is currently running.
+ * Exactly one of the public methods returns <code>true</code>. Each executes quickly.
+ * Useful with {@literal @}Assumption annotations to limit unit tests to specific test platforms.
+ *
  * @author m.t.b.carroll@dundee.ac.uk
  * @since 5.0
  */
-public class PlatformAssumptions {
+
+public class CurrentPlatform {
     /**
-     * A enumeration of operating systems under which tests may be occurring.
-     * Some file path tests are platform-specific.
+     * A enumeration of operating systems under which OMERO may run.
      * 
      * @author m.t.b.carroll@dundee.ac.uk
      * @since 5.0
@@ -55,15 +59,22 @@ public String toString() {
     private static final OperatingSystem os;
 
     static {
+        final Logger logger = LoggerFactory.getLogger(CurrentPlatform.class);
+
         final String osName = System.getProperty("os.name");
         if (osName.startsWith("Windows "))
             os = OperatingSystem.WINDOWS;
         else if (osName.equals("Linux"))
             os = OperatingSystem.LINUX;
         else if (osName.equals("Mac OS X"))
             os = OperatingSystem.MAC;
-        else
+        else {
             os = null;
+            logger.warn("failed to recognize current operating system");
+        }
+        if (os != null && logger.isDebugEnabled()) {
+            logger.debug("recognized current operating system as being " + os);
+        }
     }
 
     /**
@@ -86,4 +97,11 @@ public static boolean isLinux() {
     public static boolean isMacOSX() {
         return os == OperatingSystem.MAC;
     }
+
+    /**
+     * @return if this platform is unidentified
+     */
+    public static boolean isUnknown() {
+        return os == null;
+    }
 }

components/blitz/test/ome/services/blitz/test/utests/FilePathRestrictionsTest.java

@@ -19,14 +19,21 @@
 
 package ome.services.blitz.test.utests;
 
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 
+import ome.services.blitz.repo.path.FilePathRestrictionInstance;
 import ome.services.blitz.repo.path.FilePathRestrictions;
+import ome.services.blitz.util.CurrentPlatform;
+
+import nl.javadude.assumeng.Assumption;
+import nl.javadude.assumeng.AssumptionListener;
 
 import org.apache.commons.collections.CollectionUtils;
 import org.testng.Assert;
+import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
 import com.google.common.collect.HashMultimap;
@@ -40,6 +47,7 @@
  */
 
 @Test(groups = { "fs" })
+@Listeners(AssumptionListener.class)
 public class FilePathRestrictionsTest {
 
     /**
@@ -51,7 +59,7 @@ public void testCombineNoRules() {
     }
 
     /**
-     * Test that one may with rule sets created with mostly nulls.
+     * Test that one may create rule sets using mostly nulls.
      */
     @Test
     public void testNullSafety() {
@@ -378,4 +386,68 @@ public void testCyclicTransformationCombination() {
                 new FilePathRestrictions(transformationMatrix, null, null, null, ImmutableSet.of('A'));
         FilePathRestrictions.combineFilePathRestrictions(rules); 
     }
+
+    /**
+     * Assert that collections contain exactly the same elements, regardless of ordering.
+     * @param actual the actual elements
+     * @param expected the expected elements
+     */
+    private static <X> void assertEqualsNoOrder(Collection<X> actual, Collection<X> expected) {
+        final HashSet<X> remaining = new HashSet<X>(actual);
+        for (final X expectedItem : expected) {
+            Assert.assertTrue(remaining.remove(expectedItem), "collections must contain the same elements");
+        }
+        Assert.assertTrue(remaining.isEmpty(), "collections must contain the same elements");
+    }
+
+    /**
+     * Assert that file path restriction rules are identical in effect.
+     * @param actual the actual rules
+     * @param expected the expected rules
+     */
+    private static void assertSameRules(FilePathRestrictions actual, FilePathRestrictions expected) {
+        assertEqualsNoOrder(actual.transformationMatrix.entries(), expected.transformationMatrix.entries());
+        assertEqualsNoOrder(actual.unsafePrefixes, expected.unsafePrefixes);
+        assertEqualsNoOrder(actual.unsafeSuffixes, expected.unsafeSuffixes);
+        assertEqualsNoOrder(actual.unsafeNames, expected.unsafeNames);
+        assertEqualsNoOrder(actual.safeCharacters, expected.safeCharacters);
+        Assert.assertEquals(actual.safeCharacter, expected.safeCharacter);
+        assertEqualsNoOrder(actual.transformationMap.entrySet(), expected.transformationMap.entrySet());
+    }
+
+    /**
+     * On Microsoft Windows, test that the applicable rules are those for Microsoft Windows.
+     */
+    @Test
+    @Assumption(methods = {"isWindows"}, methodClass = CurrentPlatform.class)
+    public void testUnsafeCharacterUnsafetyWindows() {
+        assertSameRules(FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.LOCAL_REQUIRED),
+                        FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.WINDOWS_REQUIRED));
+        assertSameRules(FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.LOCAL_OPTIONAL),
+                        FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.WINDOWS_OPTIONAL));
+    }
+
+    /**
+     * On Linux, test that the applicable rules are those for UNIX-like platforms.
+     */
+    @Test
+    @Assumption(methods = {"isLinux"}, methodClass = CurrentPlatform.class)
+    public void testUnsafeCharacterUnsafetyLinux() {
+        assertSameRules(FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.LOCAL_REQUIRED),
+                        FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.UNIX_REQUIRED));
+        assertSameRules(FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.LOCAL_OPTIONAL),
+                        FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.UNIX_OPTIONAL));
+    }
+
+    /**
+     * On Apple Mac OS X, test that the applicable rules are those for UNIX-like platforms.
+     */
+    @Test
+    @Assumption(methods = {"isMacOSX"}, methodClass = CurrentPlatform.class)
+    public void testUnsafeCharacterUnsafetyMacOSX() {
+        assertSameRules(FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.LOCAL_REQUIRED),
+                        FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.UNIX_REQUIRED));
+        assertSameRules(FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.LOCAL_OPTIONAL),
+                        FilePathRestrictionInstance.getFilePathRestrictions(FilePathRestrictionInstance.UNIX_OPTIONAL));
+    }
 }

components/blitz/test/ome/services/blitz/test/utests/MakePathComponentSafeTest.java

@@ -33,6 +33,7 @@
 import ome.services.blitz.repo.path.FilePathRestrictionInstance;
 import ome.services.blitz.repo.path.FilePathRestrictions;
 import ome.services.blitz.repo.path.MakePathComponentSafe;
+import ome.services.blitz.util.CurrentPlatform;
 import omero.util.TempFileManager;
 
 import nl.javadude.assumeng.Assumption;
@@ -185,7 +186,7 @@ private void testUnsafeCharacterUnsafety(FilePathRestrictionInstance ruleId) thr
      * @throws IOException unexpected
      */
     @Test
-    @Assumption(methods = {"isWindows"}, methodClass = PlatformAssumptions.class)
+    @Assumption(methods = {"isWindows"}, methodClass = CurrentPlatform.class)
     public void testUnsafeCharacterUnsafetyWindows() throws IOException {
         testUnsafeCharacterUnsafety(FilePathRestrictionInstance.WINDOWS_REQUIRED);
     }
@@ -195,7 +196,7 @@ public void testUnsafeCharacterUnsafetyWindows() throws IOException {
      * @throws IOException unexpected
      */
     @Test
-    @Assumption(methods = {"isLinux"}, methodClass = PlatformAssumptions.class)
+    @Assumption(methods = {"isLinux"}, methodClass = CurrentPlatform.class)
     public void testUnsafeCharacterUnsafetyLinux() throws IOException {
         testUnsafeCharacterUnsafety(FilePathRestrictionInstance.UNIX_REQUIRED);
     }
@@ -205,11 +206,21 @@ public void testUnsafeCharacterUnsafetyLinux() throws IOException {
      * @throws IOException unexpected
      */
     @Test
-    @Assumption(methods = {"isMacOSX"}, methodClass = PlatformAssumptions.class)
+    @Assumption(methods = {"isMacOSX"}, methodClass = CurrentPlatform.class)
     public void testUnsafeCharacterUnsafetyMacOSX() throws IOException {
         testUnsafeCharacterUnsafety(FilePathRestrictionInstance.UNIX_REQUIRED);
     }
 
+    /**
+     * Test that one of the operating-system-specific tests for the unsafety of unsafe characters
+     * did actually execute because the current operating system was actually recognized.
+     */
+    @Test
+    @Assumption(methods = {"isUnknown"}, methodClass = CurrentPlatform.class)
+    public void testPlatformTestExecuted() {
+        Assert.fail("one of the operating-system-specific tests should have executed");
+    }
+
     /**
      * Test that data can be stored in files named using sanitized unsafe characters.
      * @throws IOException unexpected

etc/omero.properties

@@ -216,9 +216,13 @@ omero.fs.repo.path=%user%_%userId%/%year%-%month%/%day%/%time%
 #  - Windows optional
 #  - UNIX required
 #  - UNIX optional
+#  - local required
+#  - local optional
 # Minimally, the "required" appropriate for the server is recommended.
-# Also applying optional rules may make sysadmin tasks easier,
+# Also applying "optional" rules may make sysadmin tasks easier,
 # but may be more burdensome for users who name their files oddly.
+# "local" means "Windows" or "UNIX" depending on the local platform,
+# the latter being applied for Linux and Mac OS X.
 omero.fs.repo.path_rules=Windows required, UNIX required
 
 # Checksum algorithms supported by the server for new file uploads,