Address review feedback for sync command

- Fix mount point matching: select longest prefix (most specific) match
- Fix root mount matching for paths like /var/log
- Add null device guard in flushFileSystem
- Print note when device-level flush is skipped (ApiNotFoundException)
- Add description attribute to path argument in XML descriptor
- Extract findMountPoint into testable public static method
- Add unit tests for mount point matching logic (5 tests, all pass)

Author: opencode-agent[bot]

fs/build-tests.xml

@@ -40,6 +40,7 @@ help         Output these messages
             <test name="org.jnode.test.driver.bus.ide.IDEDriveDescriptorTest" todir="${basedir}/build/reports/junit"/>
             <test name="org.jnode.test.fs.filesystem.FSTestSuite" todir="${basedir}/build/reports/junit"/>
             <test name="org.jnode.test.fs.driver.tests.BlockDeviceAPITest" todir="${basedir}/build/reports/junit"/>
+            <test name="org.jnode.test.fs.command.SyncCommandTest" todir="${basedir}/build/reports/junit"/>
         </junit>
 	</target>
 

fs/descriptors/org.jnode.fs.command.xml

@@ -63,7 +63,7 @@
     <syntax alias="sync">
       <empty description="Flush all mounted filesystems to disk"/>
       <sequence description="Flush the filesystem at a specific path">
-        <argument argLabel="path"/>
+        <argument argLabel="path" description="the path of the filesystem to flush"/>
       </sequence>
     </syntax>
   </extension>

fs/src/commands/org/jnode/fs/command/SyncCommand.java

@@ -59,19 +59,13 @@ public void execute() throws Exception {
 
         if (argPath.isSet()) {
             String path = argPath.getValue().getCanonicalPath();
-            FileSystem<?> target = null;
-            String mountPoint = null;
-            for (Map.Entry<String, FileSystem<?>> e : fss.getMountPoints().entrySet()) {
-                if (path.equals(e.getKey()) || path.startsWith(e.getKey() + "/")) {
-                    target = e.getValue();
-                    mountPoint = e.getKey();
-                }
-            }
-            if (target == null) {
+            Map.Entry<String, FileSystem<?>> match =
+                findMountPoint(fss.getMountPoints(), path);
+            if (match == null) {
                 err.println("No filesystem mounted at " + path);
                 exit(1);
             }
-            flushFileSystem(target, mountPoint, out, err);
+            flushFileSystem(match.getValue(), match.getKey(), out, err);
         } else {
             int errors = 0;
             for (Map.Entry<String, FileSystem<?>> e : fss.getMountPoints().entrySet()) {
@@ -86,17 +80,51 @@ public void execute() throws Exception {
         }
     }
 
+    public static Map.Entry<String, FileSystem<?>> findMountPoint(
+            Map<String, FileSystem<?>> mountPoints, String path) {
+        FileSystem<?> target = null;
+        String mountPoint = null;
+        for (Map.Entry<String, FileSystem<?>> e : mountPoints.entrySet()) {
+            String key = e.getKey();
+            if (path.equals(key) || path.startsWith(key + "/") ||
+                (key.length() == 1 && path.length() > 0)) {
+                if (target == null || e.getKey().length() > mountPoint.length()) {
+                    target = e.getValue();
+                    mountPoint = e.getKey();
+                }
+            }
+        }
+        if (target == null) {
+            return null;
+        }
+        final FileSystem<?> t = target;
+        final String mp = mountPoint;
+        return new Map.Entry<String, FileSystem<?>>() {
+            public String getKey() { return mp; }
+            public FileSystem<?> getValue() { return t; }
+            public FileSystem<?> setValue(FileSystem<?> v) { throw new UnsupportedOperationException(); }
+        };
+    }
+
     private void flushFileSystem(FileSystem<?> fs, String mountPoint,
             PrintWriter out, PrintWriter err) throws IOException {
         if (fs instanceof AbstractFileSystem) {
             ((AbstractFileSystem<?>) fs).flush();
         }
         Device device = fs.getDevice();
-        try {
-            BlockDeviceAPI api = device.getAPI(BlockDeviceAPI.class);
-            api.flush();
-        } catch (ApiNotFoundException ex) {
+        boolean deviceFlushed = false;
+        if (device != null) {
+            try {
+                BlockDeviceAPI api = device.getAPI(BlockDeviceAPI.class);
+                api.flush();
+                deviceFlushed = true;
+            } catch (ApiNotFoundException ex) {
+            }
+        }
+        out.print("synced " + mountPoint);
+        if (!deviceFlushed) {
+            out.print(" (device flush skipped)");
         }
-        out.println("synced " + mountPoint);
+        out.println();
     }
 }

fs/src/test/org/jnode/test/fs/command/SyncCommandTest.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) 2003-2015 JNode.org
+ *
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this library; If not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+package org.jnode.test.fs.command;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import junit.framework.TestCase;
+
+import org.jnode.driver.Device;
+import org.jnode.fs.FSEntry;
+import org.jnode.fs.FileSystem;
+import org.jnode.fs.FileSystemType;
+import org.jnode.fs.command.SyncCommand;
+
+public class SyncCommandTest extends TestCase {
+
+    private Map<String, FileSystem<?>> mountPoints;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        mountPoints = new HashMap<String, FileSystem<?>>();
+        mountPoints.put("/", new StubFileSystem("/"));
+        mountPoints.put("/home", new StubFileSystem("/home"));
+        mountPoints.put("/home/user", new StubFileSystem("/home/user"));
+    }
+
+    public void testExactMountPointMatch() {
+        Map.Entry<String, FileSystem<?>> result =
+            SyncCommand.findMountPoint(mountPoints, "/home");
+        assertNotNull(result);
+        assertEquals("/home", result.getKey());
+    }
+
+    public void testLongestPrefixMatch() {
+        Map.Entry<String, FileSystem<?>> result =
+            SyncCommand.findMountPoint(mountPoints, "/home/user/docs");
+        assertNotNull(result);
+        assertEquals("/home/user", result.getKey());
+    }
+
+    public void testNoMatchingMountPoint() {
+        Map<String, FileSystem<?>> noRoot = new HashMap<String, FileSystem<?>>();
+        noRoot.put("/home", new StubFileSystem("/home"));
+        noRoot.put("/home/user", new StubFileSystem("/home/user"));
+        Map.Entry<String, FileSystem<?>> result =
+            SyncCommand.findMountPoint(noRoot, "/var/log");
+        assertNull(result);
+    }
+
+    public void testRootMatch() {
+        Map.Entry<String, FileSystem<?>> result =
+            SyncCommand.findMountPoint(mountPoints, "/var/log");
+        assertNotNull(result);
+        assertEquals("/", result.getKey());
+    }
+
+    public void testExactRootMatch() {
+        Map.Entry<String, FileSystem<?>> result =
+            SyncCommand.findMountPoint(mountPoints, "/");
+        assertNotNull(result);
+        assertEquals("/", result.getKey());
+    }
+
+    private static class StubFileSystem implements FileSystem<FSEntry> {
+        private final String mp;
+
+        StubFileSystem(String mp) {
+            this.mp = mp;
+        }
+
+        public FileSystemType<? extends FileSystem<FSEntry>> getType() {
+            return null;
+        }
+
+        public Device getDevice() {
+            return null;
+        }
+
+        public FSEntry getRootEntry() throws IOException {
+            return null;
+        }
+
+        public boolean isReadOnly() {
+            return false;
+        }
+
+        public void close() throws IOException {
+        }
+
+        public boolean isClosed() {
+            return false;
+        }
+
+        public long getTotalSpace() throws IOException {
+            return 0;
+        }
+
+        public long getFreeSpace() throws IOException {
+            return 0;
+        }
+
+        public long getUsableSpace() throws IOException {
+            return 0;
+        }
+
+        public String getVolumeName() throws IOException {
+            return mp;
+        }
+    }
+}