Merge pull request #16977 from iterate-ch/bugfix/GH-16976

dkocher · web-flow · commit df655732acab · 2025-03-24T22:03:06.000+01:00
Allow to remove metadata
diff --git a/azure/src/test/java/ch/cyberduck/core/azure/AzureMetadataFeatureTest.java b/azure/src/test/java/ch/cyberduck/core/azure/AzureMetadataFeatureTest.java
@@ -24,12 +24,16 @@ public void testSetMetadata() throws Exception {
         final Path container = new Path("cyberduck", EnumSet.of(Path.Type.directory, Path.Type.volume));
         final Path test = new Path(container, new AlphanumericRandomStringService().random(), EnumSet.of(Path.Type.file));
         new AzureTouchFeature(session, null).touch(test, new TransferStatus());
+        final TransferStatus status = new TransferStatus();
         final String v = new AlphanumericRandomStringService().random();
-        new AzureMetadataFeature(session, null).setMetadata(test, Collections.singletonMap("Test", v));
-        final Map<String, String> metadata = new AzureMetadataFeature(session, null).getMetadata(test);
+        final AzureMetadataFeature feature = new AzureMetadataFeature(session, null);
+        feature.setMetadata(test, status.setMetadata(Collections.singletonMap("Test", v)));
+        final Map<String, String> metadata = feature.getMetadata(test);
         assertFalse(metadata.isEmpty());
         assertTrue(metadata.containsKey("Test"));
         assertEquals(v, metadata.get("Test"));
+        feature.setMetadata(test, status.setMetadata(Collections.emptyMap()));
+        assertFalse(feature.getMetadata(test).containsKey("Test"));
         new AzureDeleteFeature(session, null).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 
diff --git a/googledrive/src/main/java/ch/cyberduck/core/googledrive/DriveAttributesFinderFeature.java b/googledrive/src/main/java/ch/cyberduck/core/googledrive/DriveAttributesFinderFeature.java
@@ -128,6 +128,9 @@ public PathAttributes toAttributes(final File f) {
                         .getBytes(Charset.defaultCharset()).length);
             }
         }
+        if(null != f.getProperties()) {
+            attributes.setMetadata(f.getProperties());
+        }
         return attributes;
     }
 }
diff --git a/googledrive/src/main/java/ch/cyberduck/core/googledrive/DriveMetadataFeature.java b/googledrive/src/main/java/ch/cyberduck/core/googledrive/DriveMetadataFeature.java
@@ -23,10 +23,13 @@
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 
 import com.google.api.services.drive.model.File;
 
+import static com.google.api.client.util.Data.NULL_STRING;
+
 public class DriveMetadataFeature implements Metadata {
 
     private final DriveSession session;
@@ -63,10 +66,18 @@ public void setMetadata(final Path file, final TransferStatus status) throws Bac
         try {
             final String fileid = this.fileid.getFileId(file);
             final File body = new File();
-            body.setProperties(status.getMetadata());
-            final File properties = session.getClient().files().update(fileid, body).setFields("properties").
-                    setSupportsAllDrives(HostPreferencesFactory.get(session.getHost()).getBoolean("googledrive.teamdrive.enable")).execute();
-            status.setResponse(new DriveAttributesFinderFeature(session, this.fileid).toAttributes(properties));
+            final Map<String, String> properties = new HashMap<>(status.getMetadata());
+            for(String key : file.attributes().getMetadata().keySet()) {
+                if(!properties.containsKey(key)) {
+                    // Entries with null values are cleared in update and copy requests
+                    properties.put(key, NULL_STRING);
+                }
+            }
+            body.setProperties(properties);
+            status.setResponse(new DriveAttributesFinderFeature(session, this.fileid).toAttributes(
+                    session.getClient().files().update(fileid, body).setFields("properties").
+                            setSupportsAllDrives(HostPreferencesFactory.get(session.getHost()).getBoolean("googledrive.teamdrive.enable")).execute()
+            ));
         }
         catch(IOException e) {
             throw new DriveExceptionMappingService(fileid).map("Failure to write attributes of {0}", e, file);
diff --git a/googledrive/src/test/java/ch/cyberduck/core/googledrive/DriveMetadataFeatureTest.java b/googledrive/src/test/java/ch/cyberduck/core/googledrive/DriveMetadataFeatureTest.java
@@ -29,6 +29,7 @@
 
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.Map;
 import java.util.UUID;
 
 import static ch.cyberduck.core.googledrive.DriveHomeFinderService.MYDRIVE_FOLDER;
@@ -39,14 +40,20 @@ public class DriveMetadataFeatureTest extends AbstractDriveTest {
 
     @Test
     public void setMetadata() throws Exception {
-        final Path home = DriveHomeFinderService.MYDRIVE_FOLDER;
-        final Path test = new Path(home, UUID.randomUUID().toString(), EnumSet.of(Path.Type.file));
+        final Path test = new Path(DriveHomeFinderService.MYDRIVE_FOLDER, UUID.randomUUID().toString(), EnumSet.of(Path.Type.file));
         final DriveFileIdProvider fileid = new DriveFileIdProvider(session);
-        new DriveTouchFeature(session, fileid).touch(test, new TransferStatus());
-        assertEquals(Collections.emptyMap(), new DriveMetadataFeature(session, fileid).getMetadata(test));
-        new DriveMetadataFeature(session, fileid).setMetadata(test, Collections.singletonMap("test", "t"));
-        assertEquals(Collections.singletonMap("test", "t"), new DriveMetadataFeature(session, fileid).getMetadata(test));
-        new DriveDeleteFeature(session, fileid).delete(Collections.<Path>singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());
+        final TransferStatus status = new TransferStatus();
+        new DriveTouchFeature(session, fileid).touch(test, status);
+        final DriveMetadataFeature feature = new DriveMetadataFeature(session, fileid);
+        assertEquals(Collections.emptyMap(), feature.getMetadata(test));
+        feature.setMetadata(test, Collections.singletonMap("test", "t"));
+        final Map<String, String> metadata = feature.getMetadata(test);
+        assertEquals(Collections.singletonMap("test", "t"), metadata);
+        test.attributes().setMetadata(metadata);
+        feature.setMetadata(test, status.setMetadata(Collections.emptyMap()));
+        assertFalse(status.getResponse().getMetadata().containsKey("test"));
+        assertFalse(feature.getMetadata(test).containsKey("test"));
+        new DriveDeleteFeature(session, fileid).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 
     @Test
diff --git a/googlestorage/src/test/java/ch/cyberduck/core/googlestorage/GoogleStorageMetadataFeatureTest.java b/googlestorage/src/test/java/ch/cyberduck/core/googlestorage/GoogleStorageMetadataFeatureTest.java
@@ -57,6 +57,8 @@ public void testGetMetadataFile() throws Exception {
         final Map<String, String> get = feature.getMetadata(test);
         assertTrue(get.containsKey("k"));
         assertEquals("v", get.get("k"));
+        feature.setMetadata(test, status.setMetadata(Collections.emptyMap()));
+        assertTrue(feature.getMetadata(test).isEmpty());
         new GoogleStorageDeleteFeature(session).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 
diff --git a/openstack/src/test/java/ch/cyberduck/core/openstack/SwiftMetadataFeatureTest.java b/openstack/src/test/java/ch/cyberduck/core/openstack/SwiftMetadataFeatureTest.java
@@ -52,12 +52,15 @@ public void testSetMetadata() throws Exception {
         final Path test = new Path(container, new AlphanumericRandomStringService().random(), EnumSet.of(Path.Type.file));
         new SwiftTouchFeature(session, new SwiftRegionService(session)).touch(test, new TransferStatus().setMime("text/plain"));
         final String v = UUID.randomUUID().toString();
-        new SwiftMetadataFeature(session).setMetadata(test, Collections.<String, String>singletonMap("Test", v));
-        final Map<String, String> metadata = new SwiftMetadataFeature(session).getMetadata(test);
+        final SwiftMetadataFeature feature = new SwiftMetadataFeature(session);
+        feature.setMetadata(test, Collections.singletonMap("Test", v));
+        final Map<String, String> metadata = feature.getMetadata(test);
         assertFalse(metadata.isEmpty());
         assertTrue(metadata.containsKey("X-Object-Meta-Test"));
         assertEquals(v, metadata.get("X-Object-Meta-Test"));
         assertEquals("text/plain", metadata.get("Content-Type"));
+        feature.setMetadata(test, Collections.emptyMap());
+        assertFalse(feature.getMetadata(test).containsKey("Test"));
         new SwiftDeleteFeature(session).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 }
diff --git a/pom.xml b/pom.xml
@@ -88,7 +88,7 @@
         <log4j-version>2.24.3</log4j-version>
         <bouncycastle-version>1.80</bouncycastle-version>
         <httpclient-version>4.5.14</httpclient-version>
-        <google-http-client.version>1.42.3</google-http-client.version>
+        <google-http-client.version>1.46.3</google-http-client.version>
         <jwt.version>4.5.0</jwt.version>
         <ikvm-version>8.0.312.b07-8</ikvm-version>
         <java-native-dll>8u312b07</java-native-dll>
diff --git a/s3/src/test/java/ch/cyberduck/core/s3/S3MetadataFeatureTest.java b/s3/src/test/java/ch/cyberduck/core/s3/S3MetadataFeatureTest.java
@@ -55,7 +55,8 @@ public void testGetMetadataFile() throws Exception {
         new S3TouchFeature(session, acl).touch(test, new TransferStatus()
                 .setMetadata(Collections.singletonMap("app", "cyberduck"))
                 .setMime("text/plain"));
-        final Map<String, String> metadata = new S3MetadataFeature(session, acl).getMetadata(test);
+        final S3MetadataFeature feature = new S3MetadataFeature(session, acl);
+        final Map<String, String> metadata = feature.getMetadata(test);
         assertFalse(metadata.isEmpty());
         assertTrue(metadata.containsKey("app"));
         assertEquals("cyberduck", metadata.get("app"));
@@ -64,6 +65,8 @@ public void testGetMetadataFile() throws Exception {
         assertFalse(metadata.containsKey(Constants.KEY_FOR_USER_METADATA));
         assertFalse(metadata.containsKey(Constants.KEY_FOR_SERVICE_METADATA));
         assertFalse(metadata.containsKey(Constants.KEY_FOR_COMPLETE_METADATA));
+        feature.setMetadata(test, Collections.emptyMap());
+        assertFalse(feature.getMetadata(test).containsKey("app"));
         new S3DefaultDeleteFeature(session, acl).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 
diff --git a/webdav/src/main/java/ch/cyberduck/core/dav/DAVMetadataFeature.java b/webdav/src/main/java/ch/cyberduck/core/dav/DAVMetadataFeature.java
@@ -42,6 +42,8 @@
 import com.github.sardine.impl.SardineException;
 import com.github.sardine.util.SardineUtil;
 
+import javax.xml.namespace.QName;
+
 public class DAVMetadataFeature implements Headers {
     private static final Logger log = LogManager.getLogger(DAVMetadataFeature.class);
 
@@ -84,12 +86,20 @@ public Map<String, String> getMetadata(final Path file) throws BackgroundExcepti
     public void setMetadata(final Path file, final TransferStatus status) throws BackgroundException {
         log.debug("Write metadata {} for file {}", status, file);
         try {
-            final List<Element> props = new ArrayList<>();
+            final List<Element> setProps = new ArrayList<>();
             for(Map.Entry<String, String> entry : status.getMetadata().entrySet()) {
-                Element element = SardineUtil.createElement(
+                final Element element = SardineUtil.createElement(
                         SardineUtil.createQNameWithCustomNamespace(entry.getKey()));
                 element.setTextContent(entry.getValue());
-                props.add(element);
+                setProps.add(element);
+            }
+            final List<QName> removeProps = new ArrayList<>();
+            for(String key : file.attributes().getMetadata().keySet()) {
+                final Element element = SardineUtil.createElement(SardineUtil.createQNameWithCustomNamespace(key));
+                if(!setProps.contains(element)) {
+                    // Deleted property
+                    removeProps.add(SardineUtil.createQNameWithCustomNamespace(key));
+                }
             }
             final Map<String, String> headers;
             if(session.getFeature(Lock.class) != null && status.getLockId() != null) {
@@ -98,7 +108,7 @@ public void setMetadata(final Path file, final TransferStatus status) throws Bac
             else {
                 headers = Collections.emptyMap();
             }
-            session.getClient().patch(new DAVPathEncoder().encode(file), props, Collections.emptyList(), headers);
+            session.getClient().patch(new DAVPathEncoder().encode(file), setProps, removeProps, headers);
         }
         catch(SardineException e) {
             throw new DAVExceptionMappingService().map("Failure to write attributes of {0}", e, file);
diff --git a/webdav/src/test/java/ch/cyberduck/core/dav/DAVMetadataFeatureTest.java b/webdav/src/test/java/ch/cyberduck/core/dav/DAVMetadataFeatureTest.java
@@ -49,6 +49,9 @@ public void testSetMetadataFile() throws Exception {
         assertTrue(metadata.containsKey("Test"));
         assertEquals(v, metadata.get("Test"));
         assertEquals(status.getMetadata(), metadata);
+        test.attributes().setMetadata(metadata);
+        feature.setMetadata(test, status.setMetadata(Collections.emptyMap()));
+        assertFalse(feature.getMetadata(test).containsKey("Test"));
         new DAVDeleteFeature(session).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 
diff --git a/webdav/src/test/java/ch/cyberduck/core/dav/DAVTimestampFeatureTest.java b/webdav/src/test/java/ch/cyberduck/core/dav/DAVTimestampFeatureTest.java
@@ -47,6 +47,10 @@ public void testSetTimestamp() throws Exception {
         assertEquals(5000L, attr.getModificationDate());
         assertEquals(status.getResponse(), attr);
         assertEquals(5000L, new DefaultAttributesFinderFeature(session).find(file).getModificationDate());
+        new DAVMetadataFeature(session).setMetadata(file, status.setMetadata(Collections.singletonMap("Test", "Value")));
+        assertEquals(5000L, new DAVAttributesFinderFeature(session).find(file).getModificationDate());
+        new DAVMetadataFeature(session).setMetadata(file, status.setMetadata(Collections.emptyMap()));
+        assertEquals(5000L, new DAVAttributesFinderFeature(session).find(file).getModificationDate());
         new DAVDeleteFeature(session).delete(Collections.<Path>singletonList(file), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 

Original file line number	Diff line number	Diff line change
`@@ -128,6 +128,9 @@ public PathAttributes toAttributes(final File f) {`
`128`	`128`	`.getBytes(Charset.defaultCharset()).length);`
`129`	`129`	`}`
`130`	`130`	`}`
	`131`	`+ if(null != f.getProperties()) {`
	`132`	`+ attributes.setMetadata(f.getProperties());`
	`133`	`+ }`
`131`	`134`	`return attributes;`
`132`	`135`	`}`
`133`	`136`	`}`
Original file line number	Diff line number	Diff line change
`@@ -57,6 +57,8 @@ public void testGetMetadataFile() throws Exception {`
`57`	`57`	`final Map<String, String> get = feature.getMetadata(test);`
`58`	`58`	`assertTrue(get.containsKey("k"));`
`59`	`59`	`assertEquals("v", get.get("k"));`
	`60`	`+ feature.setMetadata(test, status.setMetadata(Collections.emptyMap()));`
	`61`	`+ assertTrue(feature.getMetadata(test).isEmpty());`
`60`	`62`	`new GoogleStorageDeleteFeature(session).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());`
`61`	`63`	`}`
`62`	`64`
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,9 @@ public void testSetMetadataFile() throws Exception {`
`49`	`49`	`assertTrue(metadata.containsKey("Test"));`
`50`	`50`	`assertEquals(v, metadata.get("Test"));`
`51`	`51`	`assertEquals(status.getMetadata(), metadata);`
	`52`	`+ test.attributes().setMetadata(metadata);`
	`53`	`+ feature.setMetadata(test, status.setMetadata(Collections.emptyMap()));`
	`54`	`+ assertFalse(feature.getMetadata(test).containsKey("Test"));`
`52`	`55`	`new DAVDeleteFeature(session).delete(Collections.singletonList(test), new DisabledLoginCallback(), new Delete.DisabledCallback());`
`53`	`56`	`}`
`54`	`57`