Skip to content

caldav + carddav: Optimized handleMultiGet with GetCalendarObjects and GetAddressObjects #194

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

Open
prasad83 wants to merge 15 commits into
base: master
Choose a base branch
from
Open

caldav + carddav: Optimized handleMultiGet with GetCalendarObjects and GetAddressObjects #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
367bda2
caldav: multi-get optimization with Backend GetCalendarObjects
prasad83 Oct 26, 2025
cae54e7
carddav: multi-get optimization with Backend GetAddressObjects
prasad83 Oct 26, 2025
1ad9c6f
Updates server_test backend
prasad83 Oct 26, 2025
8b9e077
Updated multi-get lookup failure in caldav
prasad83 Oct 26, 2025
67d35e7
Updated carddav_test backend.
prasad83 Oct 26, 2025
74bbf33
Updated variable name
prasad83 Oct 26, 2025
3315b20
Updated carddav_test backend.
prasad83 Oct 26, 2025
41ff32b
carddav: Replaced GetAddressObject with GetAddressObjects
prasad83 Nov 2, 2025
7ab6113
caldav: Replaced GetCalendarObject with GetCalendarObjects
prasad83 Nov 2, 2025
78133ac
Merge branch 'emersion:master' into master
prasad83 Nov 2, 2025
c3547d4
caldav: Updated error handling with multi-get
prasad83 Nov 2, 2025
a3a6e60
carddav: Updated error handling with multi-get
prasad83 Nov 2, 2025
03d97fb
carddav: Fixed lookups to overcome for ...range pointer trap.
prasad83 Nov 2, 2025
66ddc6f
caldav: Fixed lookups to overcome for ...range pointer trap.
prasad83 Nov 2, 2025
7dd82b0
Merge branch 'master' into master
prasad83 Nov 17, 2025
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
36 changes: 30 additions & 6 deletions caldav/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ type Backend interface {
ListCalendars(ctx context.Context) ([]Calendar, error)
GetCalendar(ctx context.Context, path string) (*Calendar, error)

GetCalendarObject(ctx context.Context, path string, req *CalendarCompRequest) (*CalendarObject, error)
GetCalendarObjects(ctx context.Context, paths []string, req *CalendarCompRequest) ([]CalendarObject, error)

ListCalendarObjects(ctx context.Context, path string, req *CalendarCompRequest) ([]CalendarObject, error)
QueryCalendarObjects(ctx context.Context, path string, query *CalendarQuery) ([]CalendarObject, error)
PutCalendarObject(ctx context.Context, path string, calendar *ical.Calendar, opts *PutCalendarObjectOptions) (*CalendarObject, error)
Expand Down Expand Up @@ -258,9 +259,30 @@ func (h *Handler) handleMultiget(ctx context.Context, w http.ResponseWriter, mul
dataReq = *decoded
}

// Prefetch all objects and index by path for quick lookup in response generation.
lookups := make(map[string]*CalendarObject)
Copy link
Owner

@emersion emersion Oct 30, 2025

Choose a reason for hiding this comment

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

Do we really need to build a lookup map? Can't we just iterate over the results?

Copy link
Contributor Author

@prasad83 prasad83 Oct 31, 2025

Choose a reason for hiding this comment

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

This is minor optimization (considering the objects could be more than 1000)

Copy link
Owner

@emersion emersion Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm suggesting to replace the multiget.Hrefs loop below when building responses with a loop over objects.

Copy link
Contributor Author

@prasad83 prasad83 Nov 2, 2025

Choose a reason for hiding this comment

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

@emersion - The lookups ensures we can match each href to its object efficiently and preserve request order without nested loops. It also gracefully handles missing paths.

This way, backend GetCalendarObjects or GetAddressObjects need to worry about the order of returned objects or missing those that are not found.


paths := make([]string, len(multiget.Hrefs))
for i, href := range multiget.Hrefs {
paths[i] = href.Path
}
if objects, err := h.Backend.GetCalendarObjects(ctx, paths, &dataReq); err != nil {
return err
} else {
for i := range objects {
lookups[objects[i].Path] = &objects[i]
}
}

// response preperation with lookup
var resps []internal.Response
for _, href := range multiget.Hrefs {
co, err := h.Backend.GetCalendarObject(ctx, href.Path, &dataReq)
var co *CalendarObject
var err error
var found bool
if co, found = lookups[href.Path]; !found {
err = internal.HTTPErrorf(http.StatusNotFound, "Couldn't find calendar object at: %s", href.Path)
}
if err != nil {
resp := internal.NewErrorResponse(href.Path, err)
resps = append(resps, *resp)
Expand Down Expand Up @@ -322,7 +344,7 @@ func (b *backend) Options(r *http.Request) (caps []string, allow []string, err e
}

var dataReq CalendarCompRequest
_, err = b.Backend.GetCalendarObject(r.Context(), r.URL.Path, &dataReq)
_, err = b.Backend.GetCalendarObjects(r.Context(), []string{r.URL.Path}, &dataReq)
if httpErr, ok := err.(*internal.HTTPError); ok && httpErr.Code == http.StatusNotFound {
return caps, []string{http.MethodOptions, http.MethodPut}, nil
} else if err != nil {
Expand All @@ -344,11 +366,13 @@ func (b *backend) HeadGet(w http.ResponseWriter, r *http.Request) error {
if r.Method != http.MethodHead {
dataReq.AllProps = true
}
co, err := b.Backend.GetCalendarObject(r.Context(), r.URL.Path, &dataReq)
cos, err := b.Backend.GetCalendarObjects(r.Context(), []string{r.URL.Path}, &dataReq)
if err != nil {
return err
}

co := &cos[0]

w.Header().Set("Content-Type", ical.MIMEType)
if co.ContentLength > 0 {
w.Header().Set("Content-Length", strconv.FormatInt(co.ContentLength, 10))
Expand Down Expand Up @@ -443,12 +467,12 @@ func (b *backend) PropFind(r *http.Request, propfind *internal.PropFind, depth i
resps = append(resps, resps_...)
}
case resourceTypeCalendarObject:
ao, err := b.Backend.GetCalendarObject(r.Context(), r.URL.Path, &dataReq)
cos, err := b.Backend.GetCalendarObjects(r.Context(), []string{r.URL.Path}, &dataReq)
if err != nil {
return nil, err
}

resp, err := b.propFindCalendarObject(r.Context(), propfind, ao)
resp, err := b.propFindCalendarObject(r.Context(), propfind, &cos[0])
if err != nil {
return nil, err
}
Expand Down
10 changes: 10 additions & 0 deletions caldav/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,16 @@ func (t testBackend) GetCalendarObject(ctx context.Context, path string, req *Ca
return nil, fmt.Errorf("Couldn't find calendar object at: %s", path)
}

func (t testBackend) GetCalendarObjects(ctx context.Context, paths []string, req *CalendarCompRequest) ([]CalendarObject, error) {
objs := make([]CalendarObject, 0)
for _, path := range paths {
if obj, err := t.GetCalendarObject(ctx, path, req); err == nil {
objs = append(objs, *obj)
}
}
return objs, nil
}

func (t testBackend) PutCalendarObject(ctx context.Context, path string, calendar *ical.Calendar, opts *PutCalendarObjectOptions) (*CalendarObject, error) {
return nil, nil
}
Expand Down
10 changes: 10 additions & 0 deletions carddav/carddav_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ func (*testBackend) GetAddressObject(ctx context.Context, path string, req *Addr
}
}

func (b *testBackend) GetAddressObjects(ctx context.Context, paths []string, req *AddressDataRequest) ([]AddressObject, error) {
addresses := make([]AddressObject, 0)
for _, path := range paths {
if ao, err := b.GetAddressObject(ctx, path, req); err == nil {
addresses = append(addresses, *ao)
}
}
return addresses, nil
}

func (b *testBackend) ListAddressObjects(ctx context.Context, path string, req *AddressDataRequest) ([]AddressObject, error) {
p := ctx.Value(addressBookPathKey).(string)
if !strings.HasPrefix(path, p) {
Expand Down
35 changes: 29 additions & 6 deletions carddav/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Backend interface {
GetAddressBook(ctx context.Context, path string) (*AddressBook, error)
CreateAddressBook(ctx context.Context, addressBook *AddressBook) error
DeleteAddressBook(ctx context.Context, path string) error
GetAddressObject(ctx context.Context, path string, req *AddressDataRequest) (*AddressObject, error)
GetAddressObjects(ctx context.Context, paths []string, req *AddressDataRequest) ([]AddressObject, error)
ListAddressObjects(ctx context.Context, path string, req *AddressDataRequest) ([]AddressObject, error)
QueryAddressObjects(ctx context.Context, path string, query *AddressBookQuery) ([]AddressObject, error)
PutAddressObject(ctx context.Context, path string, card vcard.Card, opts *PutAddressObjectOptions) (*AddressObject, error)
Expand Down Expand Up @@ -222,9 +222,30 @@ func (h *Handler) handleMultiget(ctx context.Context, w http.ResponseWriter, mul
dataReq = *decoded
}

// Prefetch all objects and index by path for quick lookup in response generation.
lookups := make(map[string]*AddressObject)

paths := make([]string, len(multiget.Hrefs))
for i, href := range multiget.Hrefs {
paths[i] = href.Path
}
if objects, err := h.Backend.GetAddressObjects(ctx, paths, &dataReq); err != nil {
return err
} else {
for i := range objects {
lookups[objects[i].Path] = &objects[i]
}
}

// response preperation with lookup
var resps []internal.Response
for _, href := range multiget.Hrefs {
ao, err := h.Backend.GetAddressObject(ctx, href.Path, &dataReq)
var ao *AddressObject
var found bool
var err error
if ao, found = lookups[href.Path]; !found {
err = internal.HTTPErrorf(http.StatusNotFound, "Couldn't find address object at: %s", href.Path)
}
if err != nil {
resp := internal.NewErrorResponse(href.Path, err)
resps = append(resps, *resp)
Expand Down Expand Up @@ -288,7 +309,7 @@ func (b *backend) Options(r *http.Request) (caps []string, allow []string, err e
}

var dataReq AddressDataRequest
_, err = b.Backend.GetAddressObject(r.Context(), r.URL.Path, &dataReq)
_, err = b.Backend.GetAddressObjects(r.Context(), []string{r.URL.Path}, &dataReq)
if httpErr, ok := err.(*internal.HTTPError); ok && httpErr.Code == http.StatusNotFound {
return caps, []string{http.MethodOptions, http.MethodPut}, nil
} else if err != nil {
Expand All @@ -310,11 +331,13 @@ func (b *backend) HeadGet(w http.ResponseWriter, r *http.Request) error {
if r.Method != http.MethodHead {
dataReq.AllProp = true
}
ao, err := b.Backend.GetAddressObject(r.Context(), r.URL.Path, &dataReq)
aos, err := b.Backend.GetAddressObjects(r.Context(), []string{r.URL.Path}, &dataReq)
if err != nil {
return err
}

ao := &aos[0]

w.Header().Set("Content-Type", vcard.MIMEType)
if ao.ContentLength > 0 {
w.Header().Set("Content-Length", strconv.FormatInt(ao.ContentLength, 10))
Expand Down Expand Up @@ -409,12 +432,12 @@ func (b *backend) PropFind(r *http.Request, propfind *internal.PropFind, depth i
resps = append(resps, resps_...)
}
case resourceTypeAddressObject:
ao, err := b.Backend.GetAddressObject(r.Context(), r.URL.Path, &dataReq)
aos, err := b.Backend.GetAddressObjects(r.Context(), []string{r.URL.Path}, &dataReq)
if err != nil {
return nil, err
}

resp, err := b.propFindAddressObject(r.Context(), propfind, ao)
resp, err := b.propFindAddressObject(r.Context(), propfind, &aos[0])
if err != nil {
return nil, err
}
Expand Down