dupondje / PHP-Push-2

Z-Push Fork With CalDAV/LDAP Support
GNU Affero General Public License v3.0
67 stars 24 forks source link

CardDAV support for owncloud / sabredav plus external IMAP provider #83

Closed arved closed 11 years ago

arved commented 11 years ago

redone to not replace the original carddav.php, but to use as an alternative

Based on the CardDAV code of ziirish there is an working CardDAV backend for owncloud / SabreDAV. (see issue #80 / #76 / #47/ #71 )

to support my szenario there is a need to support external email provider with complete email address as loginname Owncloud, can not handle Email adresses as usernamens. To use PHP-Push-2 in this requirement I've added some code

exiva commented 11 years ago

Creating a contact with a image is broken. The image data shows up as a note in the card.

AddressBook

Here's the vCard file

Edit: Also, syncing a contact from ownCloud to a Phone with an image will result in the contact being discarded and not synced to the phone.

Basically, contact images are broken. Was a file size problem.

arved commented 11 years ago

the vcard shows up the same way directly imported to my android device. So I assume it's probably more a problem in the vcard than in the code. this is how the vcard shows up in plaintext: vcardtxt

it looks like there is a linebreak missing before the PHOTO; tag so everything in the line with NOTE: is interpreted as a note text

exiva commented 11 years ago

Yep, was fixed in efb7729

arved commented 11 years ago

the fix you provided is making problems in my environment. So I reveted the changes you made, and changed line 1303 to $data .= 'NOTE:'."\n " . str_replace('\n' , '\n ' , $message->body). "\n "; this should solve the issue

arved commented 11 years ago

@justbrain will check and test it

exiva commented 11 years ago

According to rfc2426 there should not be a \n after NOTE:, and commas and semicolons should be escaped. Though, I haven't tried to see if this breaks in this implementation.

Type example:

    NOTE:This fax number is operational 0800 to 1715
      EST\, Mon-Fri.
arved commented 11 years ago

@exiva ...and the colon must be escaped in all content fields. this is valid for all content. Obviously these special caracters are needed in some structured or multi valued fields, like CATEGORIES, ADR, ORG. so I have to add more code an do much more testing on it. this may take some time....

to be honest I did some reverse engeneering on ownclouds CardDAV / vCard behavior, because this did not behave like described in the rfcs. There are so many differenz CardDAV / vCard implementations out there, which behave slightly different, I not sure if anyone can be covered ...

I think it would be better to continue this here: https://github.com/arved/PHP-Push-2-owncloud. @exiva can you open an issue ?

exiva commented 11 years ago

I Haven't tried the new changes, but there's a typo that breaks address support.

carddav_oc5.php line 1265. if((!empty($message->$adri)) && ($messabge->$adri != '')){

should be

if((!empty($message->$adri)) && ($message->$adri != '')){

Also, can you talk about why there's a cachesync folder? This seems to break syncing on my Windows Phone (I don't have another device to test.) and duplicate contacts when enabled. When I disable it and leave it to the script can't write to the directory it works fine. I don't see it in the original carddav either.

arved commented 11 years ago

this folder is because of a missing implemtation of "sync-collection" in sabredav more on this here https://github.com/fmbiete/Z-Push-contrib/pull/1 this carddav implementation is based on the code of ziirish ...

dupondje commented 11 years ago

Hi

First of all thank you for your work on this. But some remarks.

1) Why did you choose to add a CardDAV backend, and not fix/update the current CardDAV backend? I do not like we have 2 backends for CardDAV. We should make 1 good, which works with all CardDAV servers.

2) I prefer not to do ANY changes to the existing Z-Push code. If it would be needed, please let me know. Then i'll commit a change on Z-Push itself :) If we start adding patches on Z-Push code, it will get unmaintainable to patch it every time.

3) Would it eventually be possible to create 1 pull request with just the addition of the CardDAV backend + Changes? Cause now i'm losing myself a bit in all the commits you did.

If this can be fixed, i'll be happy to merge your commit!

arved commented 11 years ago

Hi, the main issue is, sabreDAV / ownCloud does not support all carddav commands defined in the standard. So from my point of view there is a good backend - the original one. But for this special case there is a need to work around the missing / non-standard parts of sabreDAV / owncloud. This is the reason to make an additional one, hoping one day sabreDAV will fully support the standard and my one will be obsolete. The carddav backend for sabreDAV is modular, so there are two files and a few additional lines in the config files You're right the IMAP addtion should be handled seperatly. In fact this is a really small thing. Starting next week I will rework the request, to just contain the additional CardDAV backend for owncloud / SabreDAV

dupondje commented 11 years ago

Hi

I guess there can be a way to detect if its sabredav/owncloud, and then use some workarounds in the same carddav class? Cause now we would have alot of duplicate code as most of the carddav code can just be used for both.

arved commented 11 years ago

Hi, probably there is a way to do so, but I'm not a progammer, this is my frist expericence with php. Before - few years ago - I've just did some VBScript. So this is beyoned my skills. Maybe a real php programmer can help us out?

nafets227 commented 11 years ago

what about doing it in two steps: 1) submit the oc5 backend as additional one 2) rework to unify oc5 and normal backend, maybe with one or two config-switches.

I can try this; but since my time is limited I cannot promise...

arved commented 11 years ago

within the nex few daxs I will address nafets227 first point by reworking an resubmitting a singe push request.

arved commented 11 years ago

Hi, I'm struggeling with the usage of github :-( the owncloud carddav backend consists of two additional files (\PHP-Push-2-owncloud\backend\carddav_oc5.php and \PHP-Push-2-owncloud\include\carddav_iOC5.php) along with modification in two config.php files (PHP-Push-2-owncloud\config.php and PHP-Push-2-owncloud\backend\combined\config.php).

But I have no idea how to realize a pull request for only this without loosing everything else.

Can someone help me out?

nafets227 commented 11 years ago

Hi,

I have no clean solution (maybe someone else), but to workaround this I would:

if it helps I can try this for you and you can check the results before pushing to the master branch where dupondje has to accept.

arved commented 11 years ago

replaced by new pull request #91 with only the changes for sabeDAV / CardDAV / owncloud