thegrizzlylabs / sardine-android

A WebDAV library for Android
Apache License 2.0
355 stars 70 forks source link

Request made using patch(String url, Map<QName, String> setProps) has always empty body #6

Closed artemisia-absynthium closed 6 years ago

artemisia-absynthium commented 6 years ago

When trying to make a PROPPATCH request using any of the patch(...) methods, properties passed as parameter are never serialized into the request body, that results always being like this:

<?xml version="1.0" encoding="utf-8"?>
<D:propertyupdate xmlns:D="DAV:">
   <D:set/>
   <D:remove/>
</D:propertyupdate>

Inspecting the code, I noticed that either the Prop prop field in Set and Remove classes and the List<org.w3c.dom.Element> any parameter in Prop class are not annotated, that might be the problem. I tried to annotate them but when annotating the List<org.w3c.dom.Element> any parameter in Prop class I keep getting errors. I'll try to fix this and make a PR when I'm done but I wanted to let you know about the issue in the meantime in case you have a solution ready before I do.

Thank you for the library, keep up the good work!

guillaume-tgl commented 6 years ago

You're right, the Set and Remove classes are probably missing an annotation but the core issue is that I haven't found an equivalent for the JAXB's @XmlAnyElement in SimpleXml to be able to serialize and deserialize the any field in Prop. For the serialization part, I think it would be enough to create a custom Converter<org.w3c.dom.Element> handling custom elements but I don't have a solution for deserialization.

artemisia-absynthium commented 6 years ago

Thanks for the quick response! I've tried the Converter approach first but any annotation on that any field gives errors before the Converter has the chance to do any work, so, for now, I had to make a temporary workaround by creating a MyProp class that extends your Prop class, having my fields as @Elements, and adding a patch(String url, Prop setProps, Prop removeProps) method to your Sardine.java interface. I don't really think this approach is much elegant, though, that's why I'm calling it a temporary workaround 😄

guillaume-tgl commented 6 years ago

I've had some time to work on this :) Can you check my branch fix_propatch_lock_acl to see if it solves your issue?

artemisia-absynthium commented 6 years ago

Hi @guillaume-tgl thank you for getting back to me. I'll try it and let you know ASAP!

NLLAPPS commented 6 years ago

Hi, where is the branch branch fix_propatch_lock_acl ?

guillaume-tgl commented 6 years ago

My bad, I hadn't pushed it. It should be visible now.

NLLAPPS commented 6 years ago

Any plans to merge it to master? I do not have this specific issue but would like to keep it updated

guillaume-tgl commented 6 years ago

I've merged it, this issue should be fixed.