chimera: refactor PGET into abstract node

Given the fact that we would like to optimize the size of the file handles on the ".(get)" operations by not including the arguments, it now makes no sense to have the FsInode_PGET represent any concrete operation. Instead, we would like a hierarchy of get operations which have the basic read mechanism in common.

This patch represents the first of a two part refactoring.  It does the following:

1.  Changes FsInode_PGET to an abstract node.
2.  Implements the degenerate case (cursor) as a separate subclass FsInode_PCUR.
3.  Changes FsInode_CRC (checksum) to be a subclass of PGET.
4.  Adjusts the FsInodeType to reflect these changes.
5.  Modifies the JdcbFs class accordingly.

Testing: Works on both nfs 3 and nfs v4.1.

Target: master
Request: 2.7
Patch: http://rb.dcache.org/r/6407
Require-book: no
Require-notes: no
Acked-by: Tigran

Changes are invisible to user.
(cherry picked from commit 5e38de9)

Signed-off-by: alrossi <arossi@otfrid.fnal.gov>

modules/chimera/src/main/java/org/dcache/chimera/FsInodeType.java

@@ -25,10 +25,10 @@ public enum FsInodeType {
     PATHOF(4),   // the content of the inode is the absolute path of the inode
     PARENT(5),   // the content of the inode is the of the parent inode
     NAMEOF(6),   // the content of the inode is the name of the inode
-    PGET(7),     // the content of the inode is the value of requested attributes
+    PCUR(7),     // the content of the inode is the value of cursor
     PSET(8),     // by updating mtime of the inode the the defined attribute value is updated
     CONST(9),    // the content of the inode is a free form information
-    PLOC(10),     // the content of the inode is the value of requested attributes
+    PLOC(10),    // the content of the inode is the value of requested attributes
     PCRC(11);    // the content of the inode is a name-value list of checksum types and checksums
 
     private final int _id;

modules/chimera/src/main/java/org/dcache/chimera/FsInode_PCRC.java

@@ -19,7 +19,6 @@
 import java.util.Iterator;
 import java.util.Set;
 
-import org.dcache.chimera.posix.Stat;
 import org.dcache.util.Checksum;
 
 /**
@@ -28,78 +27,34 @@
  *
  * @author arossi
  */
-public class FsInode_PCRC extends FsInode {
+public class FsInode_PCRC extends FsInode_PGET {
     private String _checksum;
 
     public FsInode_PCRC(FileSystemProvider fs, String id) {
         super(fs, id, FsInodeType.PCRC);
     }
 
-    @Override
-    public int read(long pos, byte[] data, int offset, int len) {
-
+    protected String value() throws ChimeraFsException {
         if (_checksum == null) {
-            try {
-                _checksum = getChecksums();
-            } catch (ChimeraFsException e) {
-                return -1;
+            Set<Checksum> results = _fs.getInodeChecksums(this);
+            StringBuilder sb = new StringBuilder();
+
+            Iterator<Checksum> it = results.iterator();
+            if (it.hasNext()) {
+                Checksum result = it.next();
+                sb.append(result.getType()).append(":").append(
+                                result.getValue());
             }
-        }
-
-        byte[] b = (_checksum).getBytes();
-
-        /*
-         * are we still inside ?
-         */
-        if (pos > b.length) {
-            return 0;
-        }
-
-        int copyLen = Math.min(len, b.length - (int) pos);
-        System.arraycopy(b, (int) pos, data, 0, copyLen);
-
-        return copyLen;
-    }
-
-    @Override
-    public Stat stat() throws ChimeraFsException {
-
-        Stat ret = super.stat();
-        ret.setMode((ret.getMode() & 0000777) | UnixPermission.S_IFREG);
-        if (_checksum == null) {
-            _checksum = getChecksums();
-        }
-
-        ret.setSize(_checksum.length());
-        return ret;
-    }
-
-    @Override
-    public int write(long pos, byte[] data, int offset, int len) {
-        return -1;
-    }
-
-    private String getChecksums() throws ChimeraFsException {
-        Set<Checksum> results = _fs.getInodeChecksums(this);
-        StringBuilder sb = new StringBuilder();
 
-        Iterator<Checksum> it = results.iterator();
-        if (it.hasNext()) {
-            Checksum result = it.next();
-            sb.append(result.getType())
-              .append(":")
-              .append(result.getValue());
-        }
+            while (it.hasNext()) {
+                Checksum result = it.next();
+                sb.append(", ").append(result.getType()).append(":").append(
+                                result.getValue());
+            }
 
-        while (it.hasNext()) {
-            Checksum result = it.next();
-            sb.append(", ")
-              .append(result.getType())
-              .append(":")
-              .append(result.getValue());
+            sb.append(NEWLINE);
+            _checksum = sb.toString();
         }
-
-        sb.append("\n");
-        return sb.toString();
+        return _checksum;
     }
 }

modules/chimera/src/main/java/org/dcache/chimera/FsInode_PCUR.java

@@ -0,0 +1,33 @@
+/*
+ * This library is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 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 Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this program (see the file COPYING.LIB for more
+ * details); if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+package org.dcache.chimera;
+
+/**
+ * Cursor returns 0-byte string.
+ *
+ * @author arossi
+ */
+public class FsInode_PCUR extends FsInode_PGET {
+
+    public FsInode_PCUR(FileSystemProvider fs, String id) {
+        super(fs, id, FsInodeType.PCUR);
+    }
+
+    protected String value() throws ChimeraFsException {
+        return "";
+    }
+}

modules/chimera/src/main/java/org/dcache/chimera/FsInode_PGET.java

@@ -16,73 +16,32 @@
  */
 package org.dcache.chimera;
 
-import com.google.common.base.Charsets;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Map.Entry;
-
 import org.dcache.chimera.posix.Stat;
 
 /**
- * This class has been generalized to be able to provide any number of metadata
- * attribute values.  Currently these must be injected at creation.  (The only
- * one currently implemented is file locality.)
+ * This class has been generalized as a superclass.  The specific operation
+ * should be encoded as a concrete {@link FsInodeType}.
  *
  * @author arossi
  */
-public class FsInode_PGET extends FsInode {
-    private static final String NEWLINE = "\n\r";
-
-    private final Map<String, String> _metadata = new HashMap<>();
-    private String _name;
+public abstract class FsInode_PGET extends FsInode {
+    protected static final String NEWLINE = "\n\r";
 
-    public FsInode_PGET(FileSystemProvider fs, String id, String[] args) {
-        super(fs, id, FsInodeType.PGET);
-        if (args.length > 0) {
-            _name = args[0];
-            for (int i = 1; i < args.length; i++) {
-                _metadata.put(args[i], null);
-            }
-        }
+    protected FsInode_PGET(FileSystemProvider fs, String id, FsInodeType type) {
+        super(fs, id, type);
     }
 
     @Override
-    public boolean equals(Object o) {
-        if (o == this) {
-            return true;
-        }
-
-        if (!(o instanceof FsInode_PGET)) {
-            return false;
-        }
-
-        FsInode_PGET other = (FsInode_PGET)o;
+    public int read(long pos, byte[] data, int offset, int len) {
+        String value;
 
-        if ( _name != null && !_name.equals(other._name)) {
-            return false;
+        try {
+            value = value();
+        } catch (ChimeraFsException e) {
+            value = "";
         }
 
-        return super.equals(o) &&
-                        Arrays.equals(_metadata.keySet().toArray(),
-                                        other._metadata.keySet().toArray());
-    }
-
-    public synchronized String getMetadataFor(String name) {
-        return _metadata.get(name);
-    }
-
-    public String getName() {
-        return _name;
-    }
-
-    public synchronized boolean hasMetadataFor(String name) {
-        return _metadata.containsKey(name);
-    }
-
-    @Override
-    public int read(long pos, byte[] data, int offset, int len) {
-        byte[] b = metadata().getBytes();
+        byte[] b = value.getBytes();
 
         if (pos > b.length) {
             return 0;
@@ -94,48 +53,25 @@ public int read(long pos, byte[] data, int offset, int len) {
         return copyLen;
     }
 
-    public synchronized void setMetadata(String name, String value) {
-        _metadata.put(name, value);
-    }
-
     @Override
     public Stat stat() throws ChimeraFsException {
         Stat ret = super.stat();
         ret.setMode((ret.getMode() & 0000777) | UnixPermission.S_IFREG);
-        ret.setSize(metadata().length());
+        ret.setSize(value().length());
         // invalidate NFS cache
         ret.setMTime(System.currentTimeMillis());
         return ret;
     }
 
-    @Override
-    public byte[] getIdentifier() {
-        StringBuilder sb = new StringBuilder();
-
-        if(_name != null) {
-            sb.append(_name).append(':');
-        }
-
-        for (String arg : _metadata.keySet()) {
-            sb.append(arg).append(':');
-        }
-
-        return byteBase(sb.toString().getBytes(Charsets.UTF_8));
-    }
-
     @Override
     public int write(long pos, byte[] data, int offset, int len) {
         return -1;
     }
 
-    private synchronized String metadata() {
-        StringBuilder builder = new StringBuilder();
-        for (Entry<String, String> entry: _metadata.entrySet()) {
-            builder.append(entry.getKey())
-                   .append("=")
-                   .append(entry.getValue())
-                   .append(NEWLINE);
-        }
-        return builder.toString();
-    }
+    /*
+     * Concrete class should always do work here.  Implementation of specific
+     * operation will determine whether the node needs to be stateful or not.
+     * Should not return <code>null</code>.
+     */
+    protected abstract String value() throws ChimeraFsException;
 }

modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java

@@ -1062,7 +1062,7 @@ public FsInode inodeOf(FsInode parent, String name) throws ChimeraFsException {
             }
 
             if (name.equals(".(get)(cursor)")) {
-                FsInode pgetInode = new FsInode_PGET(this, parent.toString(), new String[0]);
+                FsInode pgetInode = new FsInode_PCUR(this, parent.toString());
                 if (!pgetInode.exists()) {
                     throw new FileNotFoundHimeraFsException(name);
                 }
@@ -1075,28 +1075,20 @@ public FsInode inodeOf(FsInode parent, String name) throws ChimeraFsException {
                     throw new FileNotFoundHimeraFsException(name);
                 }
 
+                inode = inodeOf(parent, cmd[1]);
+                if (!inode.exists()) {
+                    throw new FileNotFoundHimeraFsException(name);
+                }
+
                 switch(cmd[2]) {
                     case "locality":
-                        inode = inodeOf(parent, cmd[1]);
-                        if (!inode.exists()) {
-                            throw new FileNotFoundHimeraFsException(name);
-                        }
                         return getPLOC(inode.toString());
                     case "checksum":
                     case "checksums":
-                        inode = inodeOf(parent, cmd[1]);
-                        if (!inode.exists()) {
-                            throw new FileNotFoundHimeraFsException(name);
-                        }
                         return new FsInode_PCRC(this, inode.toString());
                     default:
-                        String[] args = new String[cmd.length - 1];
-                        System.arraycopy(cmd, 1, args, 0, args.length);
-                        inode = new FsInode_PGET(this, parent.toString(), args);
-                        if (!inode.exists()) {
-                            throw new FileNotFoundHimeraFsException(name);
-                        }
-                        return inode;
+                        throw new ChimeraFsException
+                            ("unsupported argument for .(get) " + cmd[2]);
                 }
             }
 
@@ -2848,8 +2840,8 @@ FsInode inodeFromBytesNew(byte[] handle) throws ChimeraFsException {
                 inode = new FsInode_PSET(this, inodeId, getArgs(opaque));
                 break;
 
-            case PGET:
-                inode = new FsInode_PGET(this, inodeId, getArgs(opaque));
+            case PCUR:
+                inode = new FsInode_PCUR(this, inodeId);
                 break;
 
             case PLOC:
@@ -2948,14 +2940,9 @@ FsInode inodeFromBytesOld(byte[] handle) throws ChimeraFsException {
                     inode = new FsInode_PSET(this, id, args);
                     break;
 
-                case PGET:
+                case PCUR:
                     id = st.nextToken();
-                    argc = st.countTokens();
-                    args = new String[argc];
-                    for (int i = 0; i < argc; i++) {
-                        args[i] = st.nextToken();
-                    }
-                    inode = new FsInode_PGET(this, id, args);
+                    inode = new FsInode_PCUR(this, id);
                     break;
 
                 case PLOC: