Skip to content
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

Support for property filters in addressbook-query REPORT #889

Closed
DeepDiver1975 opened this issue Sep 21, 2016 · 3 comments · Fixed by #903
Closed

Support for property filters in addressbook-query REPORT #889

DeepDiver1975 opened this issue Sep 21, 2016 · 3 comments · Fixed by #903

Comments

@DeepDiver1975
Copy link
Member

DeepDiver1975 commented Sep 21, 2016

Following https://tools.ietf.org/html/rfc6352#section-10.4 it is possible to limit the properties of the vcards to be returned by a addressbook-query.

So basically we should be capable to process a request like this

<?xml version="1.0" encoding="UTF-8"?>
<C:addressbook-query xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:carddav">
  <D:prop>
    <D:getetag/>
    <C:address-data>
      <C:prop name="VERSION"/>
      <C:prop name="UID"/>
      <C:prop name="NICKNAME"/>
      <C:prop name="EMAIL"/>
      <C:prop name="FN"/>
      <C:prop name="TEL"/>
    </C:address-data>
  </D:prop>
</C:addressbook-query>

@evert are you interested in accepting an PR on this?

@evert
Copy link
Member

evert commented Sep 28, 2016

Yes, I think this would be awesome! I feel that it would be good to:

  1. Consider the CalDAV variant, even if you don't implement it right away. At least think about how that would work as well, because as usual the CalDAV variant is more complex than the CardDAV variant and generally there's some benefits to doing the harder one first.
  2. Always include certain properties whether they were requested or not. Specifically, VERSION, UID should always be in there and maybe also FN. This way if a client requests a minimal object, the vcard will always still be valid. I realize this is a deviation of the spec, but I think it's worth it.

@evert evert changed the title FR: addressbook-query restrict result properties Support for property filters in addressbook-query REPORT Oct 10, 2016
DeepDiver1975 added a commit to DeepDiver1975/sabre-dav that referenced this issue Nov 15, 2016
DeepDiver1975 added a commit to DeepDiver1975/sabre-dav that referenced this issue Nov 16, 2016
DeepDiver1975 added a commit to DeepDiver1975/sabre-dav that referenced this issue Nov 16, 2016
evert added a commit that referenced this issue Nov 17, 2016
DeepDiver1975 added a commit to DeepDiver1975/sabre-dav that referenced this issue Nov 25, 2016
DeepDiver1975 added a commit to DeepDiver1975/sabre-dav that referenced this issue Nov 25, 2016
DeepDiver1975 added a commit to DeepDiver1975/sabre-dav that referenced this issue Nov 25, 2016
DeepDiver1975 added a commit to DeepDiver1975/sabre-dav that referenced this issue Nov 25, 2016
@evert
Copy link
Member

evert commented Dec 6, 2016

Hey @DeepDiver1975 ,

Thanks for all the commits. Must have been a lot of work. Especially with the subtle changes between versions. If you ever run into this again, I wanted to give you a small tip:

  • Don't bother with versions you don't care about. I don't think owncloud uses 3.0 any more, so 3.1 onwards would have been fine. I only care about adding features to older versions if anyone wants them, and even then I rather not. So basically pick ownclouds minimum sabre/dav version and go with that.
  • I find it easier to implement the feature at the oldest version, and then merge upwards. So I'd first develop it in 3.0, then merge it into 3.1, then 3.2, etc. This makes things a lot easier because things are typically more easily forwards compatible, and I this is how I literally always merge (always forward, never backward) so up to this day it should always be pretty easy to merge from any version to the next.

You might have done this too, I wasn't sure... but if you started with the newest and worked backwards it was probably a bit harder than it should have been.

cheers and thanks again!

@DeepDiver1975
Copy link
Member Author

I'm happy to help out with stuff like this. Given a decent unit test suite backports are not that hard 😉

daniellandau referenced this issue in daniellandau/3rdparty Jan 7, 2017
daniellandau referenced this issue in daniellandau/contacts Jan 7, 2017
This requires an updated SabreDav in core. The contents of address-data
are ignored before https://github.com/fruux/sabre-dav/issues/889
daniellandau referenced this issue in daniellandau/3rdparty Jan 7, 2017
This is the patch for 3.2 from
https://github.com/fruux/sabre-dav/issues/889 excluding tests

Signed-off-by: Daniel Landau <daniel@landau.fi>
daniellandau referenced this issue in daniellandau/contacts Mar 16, 2017
This requires an updated SabreDav in core. The contents of address-data
are ignored before https://github.com/fruux/sabre-dav/issues/889

Signed-off-by: Daniel Landau <daniel@landau.fi>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants