Use first_instance_start and last_instance_end to filter freebusy queries
Background
We run a fairly large DAViCal server with users in many time zones, which is configured (as is the default) to run Postgres queries under SET TIME ZONE 'UTC'
.
We often run into issues with events missing from freebusy. I have determined that most (all?) of these issues stem from limitations and bugs in the PL/pgSQL rrule_functions.sql
, many of which appear only when the postgres time zone is not the same as the local time zone - this means the test suites never catch them! I had a go at fixing this, but it was going to be some pretty harrowing code, and without tests it seemed like a poor choice.
New approach
Due to the various documented limitations of that code, I determined the best approach was to effectively deprecate rrule_event_overlaps
, instead using the first_instance_start
and last_instance_end
fields on calendar_item
to filter the SQL query*, before the PHP RRule code does the final filtering.
* rrule_event_overlaps
is still used when those fields have not been populated
The tiny problem with using first_instance_start
and last_instance_end
is that those columns are never populated! Thus this merge request also populates the columns for new/updated events, and I will at some point write a script to update all existing events. Additionally, these columns were the only remaining non-timezone-aware timestamps in the database, so a database migration was written to resolve this.
For consistency with !57 (merged), getVCalendarRange
was modified to support a fallback timezone for floating/all-day events, which is used to populate first_instance_start
and last_instance_end
. A todo item for the future: recalculate these fields when a collection's fallback timezone changes.
While I was working on this, I found myself really wanting some more traditional unit tests so I could do a TDD approach. Thus I wrote up a few PHPUnit tests which are included in cf7de16e. Hopefully these won't be too controversial an inclusion: I see them as a useful bonus for cases where you want to define the behaviour of a function before you write it, complementing the regression suite. Another thing on my todo list is to run these tests in CI - this should be fairly simple :)
There is one change to regression-suite output (not including the schema version bump): see the commit message of e449529f for details.