dcache-xrootd: refit checksum handling after xrootd4j bug fix

Motivation: When multiple checksums are defined on a pool, xrdcp fails if -C is expressed as an argument. This is because it receives the first checksum in the natural ordering, because the xrootd value for cks.type is not being received. (see https://rb.dcache.org/r/12018 and GitHub #5147). Modfication: Make code depend on fixed releases of xrootd4j. Adjust the construction of ChecksumInfo and also the use of getArgs->getPath on the pool. Result: xrdcp -C works again. Target: master Request: 5.2 Request: 5.1 Request: 5.0 Request: 4.2 Requires-notes: yes Requires-book: no Patch: https://rb.dcache.org/r/12019 Depends-on: https://rb.dcache.org/r/12018 Bug: #dCache/dcache/5147 Acked-by: Paul
dCache · Nov 19, 2019 · 847222b · 847222b
1 parent 5419fda
commit 847222b
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 50 deletions.
diff --git a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/AbstractXrootdRequestHandler.java b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/AbstractXrootdRequestHandler.java
@@ -21,17 +21,27 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.Optional;
+import java.util.Set;
+
+import org.dcache.util.Checksum;
+import org.dcache.util.Checksums;
 import org.dcache.xrootd.core.XrootdException;
 import org.dcache.xrootd.core.XrootdRequestHandler;
 import org.dcache.xrootd.protocol.XrootdProtocol;
 import org.dcache.xrootd.protocol.messages.LocateRequest;
 import org.dcache.xrootd.protocol.messages.LocateResponse;
 import org.dcache.xrootd.protocol.messages.ProtocolRequest;
 import org.dcache.xrootd.protocol.messages.ProtocolResponse;
+import org.dcache.xrootd.protocol.messages.QueryRequest;
+import org.dcache.xrootd.protocol.messages.QueryResponse;
 import org.dcache.xrootd.protocol.messages.SetRequest;
 import org.dcache.xrootd.protocol.messages.SetResponse;
 import org.dcache.xrootd.protocol.messages.XrootdResponse;
 import org.dcache.xrootd.security.SigningPolicy;
+import org.dcache.xrootd.util.ChecksumInfo;
+
+import static org.dcache.xrootd.protocol.XrootdProtocol.kXR_Unsupported;
 
 public class AbstractXrootdRequestHandler extends XrootdRequestHandler
 {
@@ -76,4 +86,43 @@ protected XrootdResponse<SetRequest> doOnSet(ChannelHandlerContext ctx, SetReque
         }
         return new SetResponse(request, "");
     }
+
+    protected QueryResponse selectChecksum(ChecksumInfo info,
+                                           Set<Checksum> checksums,
+                                           QueryRequest msg) throws XrootdException
+    {
+        if (!checksums.isEmpty()) {
+            /**
+             * xrdcp expects lower case names for checksum algorithms
+             * https://github.com/xrootd/xrootd/issues/459
+             * TODO: remove toLowerCase() call when above issue is addressed
+             */
+            Optional<String> type = info.getType();
+            if (type.isPresent()) {
+                Optional<Checksum> result = checksums.stream()
+                                                     .filter((c) -> type.get()
+                                                                        .equalsIgnoreCase(c.getType()
+                                                                                           .getName()))
+                                                     .findFirst();
+                if (result.isPresent()) {
+                    Checksum checksum = result.get();
+                    return new QueryResponse(msg,checksum.getType()
+                                                              .getName()
+                                                              .toLowerCase()
+                                                + " " + checksum.getValue());
+                }
+                throw new XrootdException(kXR_Unsupported, "Checksum exists, "
+                                + "but not of the requested type.");
+            }
+
+            Checksum checksum = Checksums.preferrredOrder().min(checksums);
+            return new QueryResponse(msg,checksum.getType()
+                                                      .getName()
+                                                      .toLowerCase()
+                                            + " " + checksum.getValue());
+        }
+        throw new XrootdException(kXR_Unsupported, "No checksum available "
+                        + "for this file.");
+    }
+
 }
diff --git a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdRedirectHandler.java b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdRedirectHandler.java
@@ -31,7 +31,6 @@
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Optional;
 import java.util.OptionalLong;
 import java.util.Set;
 import java.util.UUID;
@@ -57,7 +56,6 @@
 import org.dcache.cells.AbstractMessageCallback;
 import org.dcache.namespace.FileAttribute;
 import org.dcache.util.Checksum;
-import org.dcache.util.Checksums;
 import org.dcache.util.list.DirectoryEntry;
 import org.dcache.vehicles.PnfsListDirectoryMessage;
 import org.dcache.xrootd.core.XrootdException;
@@ -792,42 +790,15 @@ protected XrootdResponse<QueryRequest> doOnQuery(ChannelHandlerContext ctx, Quer
 
         case kXR_Qcksum:
             try {
-                ChecksumInfo info = new ChecksumInfo(msg.getArgs());
-                Set<Checksum> checksums = _door.getChecksums(createFullPath(info.getPath()),
+                ChecksumInfo info = new ChecksumInfo(msg.getPath(),
+                                                     msg.getOpaque());
+                Set<Checksum> checksums = _door.getChecksums(createFullPath(msg.getPath()),
                                                              msg.getSubject(),
                                                              _authz);
-                if (!checksums.isEmpty()) {
-                    Optional<String> type = info.getType();
-                    Optional<Checksum> result;
-
-                    if (type.isPresent()) {
-                        result = checksums.stream()
-                                          .filter((c) -> type.get()
-                                                             .equalsIgnoreCase(c.getType()
-                                                                                .getName()))
-                                          .findFirst();
-                    } else {
-                        result = Optional.of(Checksums.preferrredOrder().min(checksums));
-                    }
-
-                    /**
-                     * xrdcp expects lower case names for checksum algorithms
-                     * https://github.com/xrootd/xrootd/issues/459
-                     * TODO: remove toLowerCase() call when above issue is addressed
-                     */
-                    if (result.isPresent()) {
-                        Checksum checksum = result.get();
-                        return new QueryResponse(msg,checksum.getType().getName()
-                                                             .toLowerCase()
-                                                             + " "
-                                                             + checksum.getValue());
-                    }
-                }
+                return selectChecksum(info, checksums, msg);
             } catch (CacheException e) {
                 throw xrootdException(e);
             }
-            throw new XrootdException(kXR_Unsupported, "No checksum available for this file.");
-
         default:
             return unsupported(ctx, msg);
         }

diff --git a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/pool/XrootdPoolRequestHandler.java b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/pool/XrootdPoolRequestHandler.java
@@ -46,8 +46,6 @@
 import org.dcache.pool.movers.NettyTransferService;
 import org.dcache.pool.repository.OutOfDiskException;
 import org.dcache.pool.repository.RepositoryChannel;
-import org.dcache.util.Checksum;
-import org.dcache.util.Checksums;
 import org.dcache.util.Version;
 import org.dcache.vehicles.FileAttributes;
 import org.dcache.vehicles.XrootdProtocolInfo;
@@ -85,6 +83,7 @@
 import org.dcache.xrootd.protocol.messages.XrootdResponse;
 import org.dcache.xrootd.tpc.TpcWriteDescriptor;
 import org.dcache.xrootd.tpc.XrootdTpcInfo;
+import org.dcache.xrootd.util.ChecksumInfo;
 import org.dcache.xrootd.util.FileStatus;
 import org.dcache.xrootd.util.OpaqueStringParser;
 import org.dcache.xrootd.util.ParseException;
@@ -721,14 +720,12 @@ protected XrootdResponse<QueryRequest> doOnQuery(ChannelHandlerContext ctx, Quer
                 s.append('\n');
             }
             return new QueryResponse(msg, s.toString());
-
         case kXR_Qcksum:
-            String args = msg.getArgs();
-            int pos = args.indexOf(OPAQUE_DELIMITER);
-            if (pos == -1) {
+            String opaque = msg.getOpaque();
+            if (opaque == null) {
                 return redirectToDoor(ctx, msg);
             }
-            UUID uuid = getUuid(getOpaqueMap(args.substring(pos + 1)));
+            UUID uuid = getUuid(getOpaqueMap(opaque));
             if (uuid == null) {
                 /* The spec isn't clear about whether the path includes the opaque information or not.
                  * Thus we cannot rely on there being a uuid and without the uuid we cannot lookup the
@@ -740,17 +737,12 @@ protected XrootdResponse<QueryRequest> doOnQuery(ChannelHandlerContext ctx, Quer
             if (attributes == null) {
                 return redirectToDoor(ctx, msg);
             }
-            if (attributes.isUndefined(FileAttribute.CHECKSUM) || attributes.getChecksums().isEmpty()) {
+            if (attributes.isUndefined(FileAttribute.CHECKSUM)) {
                 throw new XrootdException(kXR_Unsupported, "No checksum available for this file.");
             }
-            Checksum checksum = Checksums.preferrredOrder().min(attributes.getChecksums());
-            /**
-             * xrdcp expects lower case names for checksum algorithms
-             * https://github.com/xrootd/xrootd/issues/459
-             * TODO: remove toLowerCase() call when above issue is addressed
-             */
-            return new QueryResponse(msg, checksum.getType().getName().toLowerCase() + " " + checksum.getValue());
-
+            return selectChecksum(new ChecksumInfo(msg.getPath(), opaque),
+                                  attributes.getChecksums(),
+                                  msg);
         default:
             return unsupported(ctx, msg);
         }

diff --git a/pom.xml b/pom.xml
@@ -67,7 +67,7 @@
         <version.smc>6.6.0</version.smc>
         <version.xerces>2.11.0</version.xerces>
         <version.jetty>9.4.18.v20190429</version.jetty>
-        <version.xrootd4j>3.5.4</version.xrootd4j>
+        <version.xrootd4j>3.5.5</version.xrootd4j>
         <version.jersey>2.28</version.jersey>
         <version.dcache-view>1.5.5</version.dcache-view>
         <version.netty>4.1.10.Final</version.netty>