emersion / go-webdav

A Go library for WebDAV, CalDAV and CardDAV
MIT License
326 stars 71 forks source link

carddav: do property filtering in match.Filter() #86

Closed bitfehler closed 2 years ago

bitfehler commented 2 years ago

I am a bit unsure whether or not this makes sense and would like to solicit your input :slightly_smiling_face:

I would say that this implementation makes the behavior of the Filter() function more correct. However, a case could be made that this performs work that e.g. backend may have already performed. Example: a hypothetical database backend might construct a database query that already returns the objects reduced to the requested properties (and/or only those matching the query). In this case, the current implementation would simply create a copy of each object with no further benefit.

However, I would argue that that is a very advanced use case, and such a backend would probably not be using the Filter() function in the first place. Also, while I could imagine such a hypothetical backend for carddav, the situation for caldav is much more complex (recursive data structures), and I think having a correct reference implementation at all would very much outweigh the performance considerations here.

Let me know how you feel about this. Also, regardless of outcome, the first commit should be merged, but I am happy to create another PR for that if we decide that this is not fit.

emersion commented 2 years ago

Yes, I think performing filtering by default makes sense. We can always expose more low-level functions later on when the need arises.