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.

Sign up for GitHub

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: PROPPATCH support for address books #147

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

carddav: PROPPATCH support for address books #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions carddav/carddav.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ type AddressBook struct {
SupportedAddressData []AddressDataType
}

type AddressBookUpdate struct {
Name *string
Description *string
}

func (ab *AddressBook) SupportsAddressData(contentType, version string) bool {
if len(ab.SupportedAddressData) == 0 {
return contentType == "text/vcard" && version == "3.0"
Expand Down
4 changes: 4 additions & 0 deletions carddav/carddav_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ func (*testBackend) CreateAddressBook(ctx context.Context, ab *AddressBook) erro
panic("TODO: implement")
}

func (*testBackend) UpdateAddressBook(ctx context.Context, path string, update *AddressBookUpdate) error {
panic("TODO: implement")
}

func (*testBackend) DeleteAddressBook(ctx context.Context, path string) error {
panic("TODO: implement")
}
Expand Down
125 changes: 100 additions & 25 deletions carddav/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Backend interface {
ListAddressBooks(ctx context.Context) ([]AddressBook, error)
GetAddressBook(ctx context.Context, path string) (*AddressBook, error)
CreateAddressBook(ctx context.Context, addressBook *AddressBook) error
UpdateAddressBook(ctx context.Context, path string, update *AddressBookUpdate) error
DeleteAddressBook(ctx context.Context, path string) error
GetAddressObject(ctx context.Context, path string, req *AddressDataRequest) (*AddressObject, error)
ListAddressObjects(ctx context.Context, path string, req *AddressDataRequest) ([]AddressObject, error)
Expand Down Expand Up @@ -284,7 +285,7 @@ func (b *backend) Options(r *http.Request) (caps []string, allow []string, err e
if b.resourceTypeAtPath(r.URL.Path) != resourceTypeAddressObject {
// Note: some clients assume the address book is read-only when
// DELETE/MKCOL are missing
return caps, []string{http.MethodOptions, "PROPFIND", "REPORT", "DELETE", "MKCOL"}, nil
return caps, []string{http.MethodOptions, "PROPFIND", "PROPPATCH", "REPORT", "DELETE", "MKCOL"}, nil
}

var dataReq AddressDataRequest
Expand All @@ -302,6 +303,8 @@ func (b *backend) Options(r *http.Request) (caps []string, allow []string, err e
http.MethodPut,
http.MethodDelete,
"PROPFIND",
// TODO PROPPATCH support for address objects
//"PROPPATCH",
}, nil
}

Expand Down Expand Up @@ -614,43 +617,115 @@ func (b *backend) propFindAllAddressObjects(ctx context.Context, propfind *inter
}

func (b *backend) PropPatch(r *http.Request, update *internal.PropertyUpdate) (*internal.Response, error) {
homeSetPath, err := b.Backend.AddressBookHomeSetPath(r.Context())
if err != nil {
return nil, err
}

resType := b.resourceTypeAtPath(r.URL.Path)
resp := internal.NewOKResponse(r.URL.Path)

if r.URL.Path == homeSetPath {
// TODO: support PROPPATCH for address books
switch resType {
case resourceTypeAddressBook:
abUpdate, err := b.propPatchAddressBook(r.Context(), update, resp)
if err != nil {
return nil, err
}
err = b.Backend.UpdateAddressBook(r.Context(), r.URL.Path, &abUpdate)
if err != nil {
return nil, err
}
case resourceTypeAddressObject:
// TODO: support PROPPATCH for address objects
return nil, internal.HTTPErrorf(http.StatusNotImplemented, "PROPPATCH for address objects not yet implemented")
default:
for _, prop := range update.Remove {
emptyVal := internal.NewRawXMLElement(prop.Prop.XMLName, nil, nil)
if err := resp.EncodeProp(http.StatusNotImplemented, emptyVal); err != nil {
return nil, err
for _, raw := range prop.Prop.Raw {
rxn, ok := raw.XMLName()
if !ok {
return nil, fmt.Errorf("failed to parse properties")
}
emptyVal := internal.NewRawXMLElement(rxn, nil, nil)
if err := resp.EncodeProp(http.StatusMethodNotAllowed, emptyVal); err != nil {
return nil, err
}
}
}
for _, prop := range update.Set {
emptyVal := internal.NewRawXMLElement(prop.Prop.XMLName, nil, nil)
if err := resp.EncodeProp(http.StatusNotImplemented, emptyVal); err != nil {
return nil, err
for _, raw := range prop.Prop.Raw {
rxn, ok := raw.XMLName()
if !ok {
return nil, fmt.Errorf("failed to parse properties")
}
emptyVal := internal.NewRawXMLElement(rxn, nil, nil)
if err := resp.EncodeProp(http.StatusMethodNotAllowed, emptyVal); err != nil {
return nil, err
}
}
}
Comment on lines 637 to 660
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to write out a multi-status here? Can't we just reply with a toplevel HTTP status instead? Something like internal.HTTPErrorf?

} else {
for _, prop := range update.Remove {
emptyVal := internal.NewRawXMLElement(prop.Prop.XMLName, nil, nil)
if err := resp.EncodeProp(http.StatusMethodNotAllowed, emptyVal); err != nil {
return nil, err
}
return resp, nil
}

func (b *backend) propPatchAddressBook(ctx context.Context, update *internal.PropertyUpdate, resp *internal.Response) (AddressBookUpdate, error) {
// TODO handle all properties
var (
result AddressBookUpdate
name internal.DisplayName
desc addressbookDescription
)
for _, prop := range update.Remove {
for _, raw := range prop.Prop.Raw {
rxn, ok := raw.XMLName()
if !ok {
return result, fmt.Errorf("failed to parse properties")
}
switch rxn {
case internal.DisplayNameName:
result.Description = new(string)
if err := resp.EncodeProp(http.StatusOK, internal.DisplayName{}); err != nil {
return result, err
}
case addressBookDescriptionName:
result.Description = new(string)
if err := resp.EncodeProp(http.StatusOK, desc); err != nil {
return result, err
}
default:
emptyVal := internal.NewRawXMLElement(rxn, nil, nil)
if err := resp.EncodeProp(http.StatusNotImplemented, emptyVal); err != nil {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would a 404 be more appropriate for the properties we don't know about?

return result, err
}
}
}
for _, prop := range update.Set {
emptyVal := internal.NewRawXMLElement(prop.Prop.XMLName, nil, nil)
if err := resp.EncodeProp(http.StatusMethodNotAllowed, emptyVal); err != nil {
return nil, err
}
for _, prop := range update.Set {
for _, raw := range prop.Prop.Raw {
rxn, ok := raw.XMLName()
if !ok {
return result, fmt.Errorf("failed to parse properties")
}
switch rxn {
case internal.DisplayNameName:
if err := raw.Decode(&name); err != nil {
return result, err
}
result.Name = &name.Name
if err := resp.EncodeProp(http.StatusOK, internal.DisplayName{}); err != nil {
return result, err
}
case addressBookDescriptionName:
if err := raw.Decode(&desc); err != nil {
return result, err
}
result.Description = &desc.Description
if err := resp.EncodeProp(http.StatusOK, desc); err != nil {
return result, err
}
default:
emptyVal := internal.NewRawXMLElement(rxn, nil, nil)
if err := resp.EncodeProp(http.StatusNotImplemented, emptyVal); err != nil {
return result, err
}
}
}
}

return resp, nil
return result, nil
}

func (b *backend) Put(r *http.Request) (*internal.Href, error) {
Expand Down