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

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

Closed
wants to merge 51 commits into from

Conversation

arved
Copy link

@arved arved commented Jul 11, 2013

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

arved and others added 30 commits June 30, 2013 18:08
to use IMAP of 3rd party providers with, email address as login name
use 3rd party IMAP privider with email address as login name
changes to work with ownCloud on qnap agains Samsung Galaxy Note2
updated to work with owncloud 5.0.7 on qnap 4.0.1 with Samsung Galaxy Note 2
updated to work with owncloud 5.0.7 on qnap 4.0.1 with Samsung Galaxy Note 2

Signed-off-by: arved <[email protected]>
Changes on device shoud now be replicated correctly
because of $type = strtolower(array_shift($fieldparts)); $types can never be "CATEGORIES"
added code by justbrain slightly adopted to the structure of the section to make things more readable
Using built in function to create FN, found in utils.php to use the FILEAS_ORDER Setting in config.php 
Introducing a new Switch to always override FN according to the Setting in config.php
Using built in function to create FN, found in utils.php to use the FILEAS_ORDER Setting in config.php 
Introducing a new Switch to always override FN according to the Setting in config.php
Android and ownCloud uses this format by default if you are creating a new contact.
Changed URLs and names.
@exiva
Copy link

exiva commented Jul 21, 2013

Yep, was fixed in efb7729

changed line 1303 to 'NOTE:'."\n "
@arved
Copy link
Author

arved commented Jul 22, 2013

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
Copy link
Author

arved commented Jul 22, 2013

@justbrain will check and test it

@exiva
Copy link

exiva commented Jul 22, 2013

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
Copy link
Author

arved commented Jul 23, 2013

@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 ?

arved added 4 commits July 23, 2013 16:15
adding addition of ;:, with escape character (\) (line 1304)for baking vCard NOTE. Removal of esc characters in Body for sync back via AS (line 1173)
adding addition of ;:, with escape character (\) (line 1304)for baking vCard NOTE. Removal of esc characters in Body for sync back via AS (line 1173)
@exiva
Copy link

exiva commented Jul 23, 2013

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
Copy link
Author

arved commented Jul 23, 2013

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

@dupondje
Copy link
Owner

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
Copy link
Author

arved commented Oct 11, 2013

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
Copy link
Owner

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
Copy link
Author

arved commented Oct 11, 2013

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
Copy link

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...

remove "missing cahrs in Email" problem for non standard compliant devices
@arved
Copy link
Author

arved commented Oct 13, 2013

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

@arved
Copy link
Author

arved commented Oct 14, 2013

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
Copy link

Hi,

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

  • create a branch in original owncloud repo
  • checkout this branch to a local filesystem
  • manually copy the three files into the local filesystem or change
    existing files to the new "should" state
    !!! this means losing all history of the changes you have in your
    repo PHP-Push-2-owncloud. But I know no other way !!!
  • add files to git
  • commit
  • push to upstream

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
Copy link
Author

arved commented Oct 15, 2013

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

@arved arved closed this Oct 15, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants