-
Notifications
You must be signed in to change notification settings - Fork 697
feat(services/webdav): Add user defined metadata support #7074
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
27cdca3 to
00f015e
Compare
|
Great work! Could you also update our behavior test configuration and enable the metadata feature for supported services? Here's the link: https://github.com/apache/opendal/tree/main/.github/services/webdav. Thank you in advance! |
00f015e to
5222f22
Compare
|
@Xuanwo Thanks for the suggestion! I did try enabling it for The This seems to be a limitation of how For now I've kept the feature available but disabled the test for these servers. Let me know if you'd like me to look into a workaround, or if we should just document this as a known limitation. |
I'm not sure if having our own namespace is the best approach. OpenDAL should ideally be capable of retrieving any user metadata. Do you think it would be a good idea to let users specify the namespace and namespace tag they prefer to use? |
|
Looks great, the only thing left here is enable the behavior tests. |
core/services/webdav/src/core.rs
Outdated
| } | ||
|
|
||
| /// Extract properties with a specific namespace prefix from the XML. | ||
| fn extract_properties_with_prefix( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems we are parsing xml here. Can we use quick-xml instead?
core/services/webdav/src/core.rs
Outdated
| } | ||
|
|
||
| /// Unescape XML entities. | ||
| fn unescape_xml(s: &str) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same.
- Add OpenDAL custom namespace constants for user-defined properties - Enable write_with_user_metadata capability in the webdav backend - Implement webdav_proppatch() method using RFC4918 PROPPATCH - Implement build_proppatch_request() to generate XML request body - Implement parse_user_metadata_from_xml() to extract user metadata - Update WebdavWriter to set user metadata after file upload - Update webdav_stat_rooted_abs_path() to read user metadata - Add unit tests for proppatch request building and metadata parsing User metadata is stored as dead properties in the OpenDAL namespace (od:) on the WebDAV server, following the DAV:propertyupdate pattern. Part of apache#4842.
The namespace declaration used 'opendal' as prefix name but elements used 'od:' prefix. This inconsistency caused XML namespace errors on servers like Nextcloud and ownCloud. Fixed by: - Renamed OPENDAL_NAMESPACE to OPENDAL_NAMESPACE_PREFIX with value 'od' - Removed separate OPENDAL_METADATA_PREFIX constant - Updated all uses to be consistent: xmlns:od and od: prefix
WebDAV servers like Nextcloud/ownCloud use dynamic namespace prefixes (e.g., x1:, x2:) instead of the 'od:' prefix we send in PROPPATCH. The XML namespace specification allows this - the namespace URI is the reliable identifier, not the prefix. This commit updates parse_user_metadata_from_xml() to: 1. Find all namespace prefixes mapped to our namespace URI 2. Extract properties using any of those prefixes Added tests for: - Dynamic namespace prefixes (Nextcloud style) - Namespace declared at root level - find_namespace_prefixes helper function
…onse A 207 status code only indicates that detailed information is available, not success or failure. The response body must be parsed to check the actual status of each property update. This commit: - Adds check_proppatch_response() to parse 207 response body and verify all property updates succeeded (status 200) - Updates writer to use this check instead of just checking HTTP status - Adds tests for success, failure, and lowercase prefix cases This should help identify if PROPPATCH requests are failing on Nextcloud/ownCloud servers.
Change the namespace from 'https://opendal.apache.org/' to 'http://owncloud.org/ns' because Nextcloud and ownCloud servers only accept properties in whitelisted namespaces. The ownCloud namespace is widely supported by: - Nextcloud (legacy support) - ownCloud - Other SabreDAV-based servers This should fix the PROPPATCH failures on Nextcloud/ownCloud.
Change the namespace back to 'https://opendal.apache.org/ns' with 'opendal' prefix. The issue was that Nextcloud/ownCloud have whitelist restrictions on their reserved namespaces (http://owncloud.org/ns and http://nextcloud.org/ns). Only specific predefined properties like 'calendar-enabled' are allowed in those namespaces. Using our own custom namespace bypasses this restriction because Nextcloud's isPropertyAllowed() returns true for any property that is NOT in their reserved namespaces.
Nextcloud and ownCloud use SabreDAV which may not return custom dead properties in PROPFIND allprop responses. According to RFC4918 Section 9.1, 'allprop does not return the value of all dead properties'. The user_metadata feature requires servers that properly return custom namespace properties in allprop responses. For now, disable this test for Nextcloud and ownCloud until we can implement a more targeted property query approach.
Allow users to specify custom namespace prefix and URI for user metadata: - user_metadata_prefix: namespace prefix (default: 'opendal') - user_metadata_uri: namespace URI (default: 'https://opendal.apache.org/ns') This enables compatibility with WebDAV servers that have whitelist restrictions on property namespaces. Users can configure the namespace to match their server's requirements. Example usage: WebdavBuilder::new() .user_metadata_prefix("my-prefix") .user_metadata_uri("http://example.com/ns")
- Replace manual XML escaping with quick_xml::escape functions - Use quick_xml::Writer for building PROPPATCH requests - Use quick_xml::Reader for parsing user metadata from XML responses - Handle GeneralRef events for proper XML entity decoding - Remove manual escape/unescape functions This addresses reviewer feedback to use quick-xml instead of manual string-based XML parsing.
7000824 to
81a3f7b
Compare
|
@Xuanwo I've implemented the configurable namespace feature you suggested ( However, the behavior tests still can't be enabled for our current WebDAV test servers:
The issue is that |
That's sad. Even if users set their own metadata, we still cannot provide it back to them in a meaningful way. I'm fine with merging this PR as it is because the work here is excellent. However, I would like to know if you have interest to explore further into how Nextcloud implements file metadata listing. For example, they provide APIs such as: ./occ metadata:get 1742
{
"my-first-meta": {
"value": "yes",
"type": "string",
"indexed": false
},
"my-second-meta": {
"value": 1234,
"type": "int",
"indexed": true
}
} |
|
Thanks! I did some research on this. Nextcloud 28+ has a dedicated metadata system using I'm interested in exploring this further! |
Which issue does this PR close?
Part of #4842.
Rationale for this change
This PR adds user-defined metadata support for the WebDAV service as part of the tracking issue #4842.
WebDAV protocol supports custom properties through the
PROPPATCHmethod (RFC4918), which allows us to store user-defined metadata as DAV dead properties.What changes are included in this PR?
enable_user_metadataconfiguration option (default:false)enable_user_metadata()builder methodwebdav_proppatch()method to set user metadata using RFC4918PROPPATCHbuild_proppatch_request()to generate XML request bodyparse_user_metadata_from_xml()to extract user metadata fromPROPFINDresponseswebdav_stat_rooted_abs_path()to parse and return user metadataWebdavWriter::write_once()to set user metadata viaPROPPATCHafter file uploadUser metadata is stored as dead properties in the OpenDAL namespace (
od:) on WebDAV servers.Note: This feature is disabled by default because not all WebDAV servers support
PROPPATCH(e.g., nginx's basic WebDAV module doesn't). Users need to explicitly enable it viaenable_user_metadata(true)if their server supports it (e.g., Apachemod_dav, Nextcloud, ownCloud).Are there any user-facing changes?
Yes. Users can now use
write_with_user_metadatawhen writing files to WebDAV storage (if enabled), and retrieve user metadata when callingstat().Example usage:
AI Usage Statement
This PR was developed with assistance from GitHub Copilot (Claude Opus 4.5).