Skip to content

Commit

Permalink
webdav: respect xattrs for COPY request
Browse files Browse the repository at this point in the history
Motivation:
if http copy is triggered by FTS, then webdav door ignores the provided
xattrs.

Modification:
Update RemoteTransferHandler to populate file attributes with xattrs
from the request params. Update Xatts#from(Map params) to accept
Map<String, String[]> as returned by ServletRequest#getParameterMap.
Update DcacheResourceFactory to use updated Xatts#from.

Result:
File transferred with http-tpc preserve extended attributes provided by FTS

Acked-by: Albert Rossi
Target: master, 8.2
Require-book: no
Require-notes: yes
(cherry picked from commit 6ad477b)
Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
  • Loading branch information
kofemann committed Feb 15, 2023
1 parent d81856a commit 63cb8b6
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 25 deletions.
9 changes: 5 additions & 4 deletions modules/common/src/main/java/org/dcache/util/Xattrs.java
Expand Up @@ -50,26 +50,27 @@ public static Map<String, String> from(URI uri) {
}

/**
* Extract extended attributes from the given {@code params} {@link Map}.
* Extract extended attributes from the given {@code params} {@link Map}. This method expected
* the params map ar value returned by {@link ServletRequest#getParameterMap}
*
* @param params The map to extract attributes from.
* @return a key-value map with extended attributes.
* @throws NullPointerException if params is {@code null}
*/
public static Map<String, String> from(Map<String, String> params) {
public static Map<String, String> from(Map<String, String[]> params) {

requireNonNull(params, "Params must be provided");

return params.entrySet()
.stream()
.filter(e -> e.getKey().startsWith(XATTR_PREFIX))
.collect(toMap(Xattrs::getName, Map.Entry::getValue));
.collect(toMap(Xattrs::getName, e -> e.getValue()[0]));
}

/**
* Convert a URL query key-value pair to the corresponding extended attribute name.
*/
private static String getName(Map.Entry<String, String> entry) {
private static String getName(Map.Entry<String, ?> entry) {
return entry.getKey().substring(XATTR_PREFIX.length());
}
}
32 changes: 21 additions & 11 deletions modules/common/src/test/java/org/dcache/util/XattrsTest.java
Expand Up @@ -5,7 +5,6 @@
import static org.hamcrest.collection.IsMapWithSize.aMapWithSize;
import static org.hamcrest.collection.IsMapWithSize.anEmptyMap;

import com.google.common.collect.ImmutableMap;
import java.net.URI;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -76,16 +75,16 @@ public void testUriXattrNullArg() {

@Test
public void testMapXattrEmpty() {
Map<String, String> params = new HashMap<>();
Map<String, String[]> params = new HashMap<>();

Map<String, String> xattrs = Xattrs.from(params);
assertThat(xattrs, anEmptyMap());
}

@Test
public void testMapXattrSingleValue() {
Map<String, String> params = ImmutableMap.of(
"xattr.key1", "value1"
Map<String, String[]> params = Map.of(
"xattr.key1", new String[]{"value1"}
);

Map<String, String> xattrs = Xattrs.from(params);
Expand All @@ -95,9 +94,9 @@ public void testMapXattrSingleValue() {

@Test
public void testMapXattrMutipleValue() {
Map<String, String> params = ImmutableMap.of(
"xattr.key1", "value1",
"xattr.key2", "value2"
Map<String, String[]> params = Map.of(
"xattr.key1", new String[] {"value1"},
"xattr.key2", new String[] {"value2"}
);

Map<String, String> xattrs = Xattrs.from(params);
Expand All @@ -108,9 +107,20 @@ public void testMapXattrMutipleValue() {

@Test
public void testMapXattrIgnoreOthers() {
Map<String, String> params = ImmutableMap.of(
"xattr.key1", "value1",
"key2", "value2"
Map<String, String[]> params = Map.of(
"xattr.key1", new String[] {"value1"},
"key2", new String[] {"value2"}
);

Map<String, String> xattrs = Xattrs.from(params);
assertThat(xattrs, aMapWithSize(1));
assertThat(xattrs, hasEntry("key1", "value1"));
}

@Test
public void testMapXattrFirstValueWins() {
Map<String, String[]> params = Map.of(
"xattr.key1", new String[] {"value1", "value2"}
);

Map<String, String> xattrs = Xattrs.from(params);
Expand All @@ -120,6 +130,6 @@ public void testMapXattrIgnoreOthers() {

@Test(expected = NullPointerException.class)
public void testMapXattrNullArg() {
Xattrs.from((Map<String, String>) null);
Xattrs.from((Map<String, String[]>) null);
}
}
Expand Up @@ -67,7 +67,6 @@
import dmg.cells.nucleus.CellMessageReceiver;
import dmg.cells.nucleus.CellPath;
import dmg.cells.services.login.LoginManagerChildrenInfo;
import io.milton.http.FileItem;
import io.milton.http.HttpManager;
import io.milton.http.Request;
import io.milton.http.ResourceFactory;
Expand Down Expand Up @@ -100,7 +99,6 @@
import java.util.Date;
import java.util.EnumSet;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -1348,14 +1346,7 @@ private Restriction roleAwareRestriction() {
* Returns request params.
*/
private static Map<String, String> getXattrsFromRequest() {
Map<String, String> m = new HashMap<>();
Map<String, FileItem> files = new HashMap<>();
try {
HttpManager.request().parseRequestParameters(m, files);
} catch (Exception e) {
throw new RuntimeException();
}
return Xattrs.from(m);
return Xattrs.from(ServletRequest.getRequest().getParameterMap());
}

/**
Expand Down
Expand Up @@ -138,6 +138,7 @@
import org.dcache.util.NetworkUtils;
import org.dcache.util.Strings;
import org.dcache.util.URIs;
import org.dcache.util.Xattrs;
import org.dcache.vehicles.FileAttributes;
import org.dcache.webdav.transfer.CopyFilter.CredentialSource;
import org.eclipse.jetty.http.HttpFields;
Expand Down Expand Up @@ -858,6 +859,10 @@ private FileAttributes resolvePath() throws ErrorResponseException {
.fileType(FileType.REGULAR)
.xattr("xdg.origin.url", _destination.toASCIIString())
.build();

Xattrs.from(ServletRequest.getRequest().getParameterMap())
.forEach(attributes::updateXattr);

try {
msg = _pnfs.createPnfsEntry(_path.toString(), attributes,
TransferManagerHandler.ATTRIBUTES_FOR_PULL);
Expand Down

0 comments on commit 63cb8b6

Please sign in to comment.