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

enh(appointments): add moar debug logging and overwrite app id #5723

Merged
merged 1 commit into from
Feb 12, 2024
+74 −31
Merged

enh(appointments): add moar debug logging and overwrite app id #5723

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
10 changes: 9 additions & 1 deletion lib/Service/Appointments/AvailabilityGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use DateTimeZone;
use OCA\Calendar\Db\AppointmentConfig;
use OCP\AppFramework\Utility\ITimeFactory;
use Psr\Log\LoggerInterface;
use function ceil;
use function max;
use function min;
Expand All @@ -39,7 +40,7 @@ class AvailabilityGenerator {
/** @var ITimeFactory */
private $timeFactory;

public function __construct(ITimeFactory $timeFactory) {
public function __construct(ITimeFactory $timeFactory, private LoggerInterface $logger) {
$this->timeFactory = $timeFactory;
}

Expand Down Expand Up @@ -79,11 +80,17 @@ public function generate(AppointmentConfig $config,

// If we reach this state then there are no available dates anymore
if ($latestEnd <= $earliestStart) {
$this->logger->debug('Appointment config ' . $config->getToken() . ' has {latestEnd} as latest end but {earliestStart} as earliest start. No slots available.', [
'latestEnd' => $latestEnd,
'earliestStart' => $earliestStart,
'app' => 'calendar-appointments'
]);
return [];
}

if (empty($config->getAvailability())) {
// No availability -> full time range is available
$this->logger->debug('Full time range available', ['app' => 'calendar-appointments']);
return [
new Interval($earliestStart, $latestEnd),
];
Expand All @@ -95,6 +102,7 @@ public function generate(AppointmentConfig $config,
$slots = $availabilityRule['slots'];

$applicableSlots = $this->filterDates($start, $slots, $timeZone);
$this->logger->debug('Found ' . count($applicableSlots) . ' applicable slot(s) after date filtering', ['app' => 'calendar-appointments']);

$intervals = [];
foreach ($applicableSlots as $slot) {
Expand Down
10 changes: 7 additions & 3 deletions lib/Service/Appointments/BookingService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,13 @@ public function confirmBooking(Booking $booking, AppointmentConfig $config): Boo
$this->mailService->sendBookingInformationEmail($booking, $config, $calendar);
$this->mailService->sendOrganizerBookingInformationEmail($booking, $config, $calendar);
} catch (ServiceException $e) {
$this->logger->info('Could not send booking emails after confirmation from user ' . $booking->getEmail(), ['exception' => $e]);
$this->logger->info('Could not send booking emails after confirmation from user ' . $booking->getEmail(), ['exception' => $e, 'app' => 'calendar-appointments']);
}

try {
$this->mailService->sendOrganizerBookingInformationNotification($booking, $config);
} catch (\InvalidArgumentException $e) {
$this->logger->warning('Could not send booking information notification after confirmation by user ' . $booking->getEmail(), ['exception' => $e]);
$this->logger->warning('Could not send booking information notification after confirmation by user ' . $booking->getEmail(), ['exception' => $e, 'app' => 'calendar-appointments']);
}

return $booking;
Expand Down Expand Up @@ -203,10 +203,13 @@ public function getAvailableSlots(AppointmentConfig $config, int $startTime, int
if ($config->getFutureLimit() !== null) {
/** @var int $maxEndTime */
$maxEndTime = time() + $config->getFutureLimit();
$this->logger->debug('Maximum end time: ' . $maxEndTime, ['app' => 'calendar-appointments']);
if ($startTime > $maxEndTime) {
$this->logger->debug('Start time is higher than maximum end time. Start time: ' . $startTime, ['app' => 'calendar-appointments']);
return [];
}
if ($endTime > $maxEndTime) {
$this->logger->debug('End time is higher than maximum end time. Setting end time to maximum end time. End time: ' . $endTime, ['app' => 'calendar-appointments']);
$endTime = $maxEndTime;
}
}
Expand All @@ -224,7 +227,8 @@ public function getAvailableSlots(AppointmentConfig $config, int $startTime, int
'availabilityIntervals' => count($availabilityIntervals),
'allPossibleSlots' => count($allPossibleSlots),
'filteredByDailyLimit' => count($filteredByDailyLimit),
'available' => count($available)
'available' => count($available),
'app' => 'calendar-appointments',
]);

return $available;
Expand Down
12 changes: 10 additions & 2 deletions lib/Service/Appointments/DailyLimitFilter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use OCA\Calendar\Db\AppointmentConfig;
use OCP\Calendar\ICalendarQuery;
use OCP\Calendar\IManager;
use Psr\Log\LoggerInterface;
use function array_filter;
use function array_values;
use function count;
Expand All @@ -36,7 +37,7 @@ class DailyLimitFilter {
/** @var IManager */
private $calendarManger;

public function __construct(IManager $calendarManger) {
public function __construct(IManager $calendarManger, private LoggerInterface $logger) {
$this->calendarManger = $calendarManger;
}

Expand All @@ -47,6 +48,10 @@ public function __construct(IManager $calendarManger) {
* @return Interval[]
*/
public function filter(AppointmentConfig $config, array $slots): array {
$this->logger->debug('Slots before daily limit filtering:' . count($slots), ['app' => 'calendar-appointments']);
if(empty($slots)) {
return [];
}
// 0. If there is no limit then we don't have to filter anything
if ($config->getDailyMax() === null) {
return $slots;
Expand Down Expand Up @@ -90,10 +95,13 @@ public function filter(AppointmentConfig $config, array $slots): array {
}

// 3. Filter out the slots that are on an unavailable day
return array_values(array_filter($slots, function (Interval $slot) use ($available): bool {
$available = array_values(array_filter($slots, function (Interval $slot) use ($available): bool {
$startOfDay = $slot->getStartAsObject()->setTime(0, 0, 0, 0);
$ts = $startOfDay->getTimestamp();
return $available[$ts];
}));

$this->logger->debug('Slots after daily limit filtering:' . count($available), ['app' => 'calendar-appointments']);
return $available;
}
}
11 changes: 9 additions & 2 deletions lib/Service/Appointments/EventConflictFilter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ public function __construct(IManager $calendarManager,
* @return Interval[]
*/
public function filter(AppointmentConfig $config, array $slots): array {
$this->logger->debug('Slots before event conflict filtering:' . count($slots), ['app' => 'calendar-appointments']);
if(empty($slots)) {
return [];
}
$query = $this->calendarManager->newQuery($config->getPrincipalUri());
foreach ($config->getCalendarFreebusyUrisAsArray() as $uri) {
$query->addSearchCalendar($uri);
Expand All @@ -63,7 +67,7 @@ public function filter(AppointmentConfig $config, array $slots): array {
$query->addType('VEVENT');
$preparationDuration = DateInterval::createFromDateString($config->getPreparationDuration() . ' seconds');
$followUpDuration = DateInterval::createFromDateString($config->getFollowupDuration() . ' seconds');
return array_filter($slots, function (Interval $slot) use ($followUpDuration, $preparationDuration, $query, $config): bool {
$available = array_filter($slots, function (Interval $slot) use ($followUpDuration, $preparationDuration, $query, $config): bool {
$query->setTimerangeStart($slot->getStartAsObject()->sub($preparationDuration));
$query->setTimerangeEnd($slot->getEndAsObject()->add($followUpDuration));

Expand All @@ -81,13 +85,16 @@ public function filter(AppointmentConfig $config, array $slots): array {

$this->logger->debug('Appointment config ' . $config->getToken() . ' is looking within {start} and {followup} in calendar {calendarUri}. Conflicting UIDs are {uids}', [
'start' => $slot->getStartAsObject()->sub($preparationDuration)->format(DateTimeInterface::ATOM),
'end' => $slot->getEndAsObject()->add($followUpDuration)->format(DateTimeInterface::ATOM),
'followup' => $slot->getEndAsObject()->add($followUpDuration)->format(DateTimeInterface::ATOM),
'calendarUri' => $config->getTargetCalendarUri(),
'uids' => implode(' : ', $uids)
]);

// If there is at least one event at this time then the slot is taken
return empty($objects);
});

$this->logger->debug('Slots after event conflict filtering:' . count($available), ['app' => 'calendar-appointments']);
return $available;
}
}
18 changes: 9 additions & 9 deletions lib/Service/Appointments/MailService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,14 @@ public function sendConfirmationEmail(Booking $booking, AppointmentConfig $confi
try {
$failed = $this->mailer->send($message);
if (count($failed) > 0) {
$this->logger->warning('Mail delivery failed for some recipients.');
$this->logger->warning('Mail delivery failed for some recipients.', ['app' => 'calendar-appointments']);
foreach ($failed as $fail) {
$this->logger->debug('Failed to deliver email to ' . $fail);
$this->logger->debug('Failed to deliver email to ' . $fail, ['app' => 'calendar-appointments']);
}
throw new ServiceException('Could not send mail for recipient(s) ' . implode(', ', $failed));
}
} catch (Exception $ex) {
$this->logger->error($ex->getMessage(), ['exception' => $ex]);
$this->logger->error($ex->getMessage(), ['exception' => $ex, 'app' => 'calendar-appointments']);
throw new ServiceException('Could not send mail: ' . $ex->getMessage(), $ex->getCode(), $ex);
}
}
Expand Down Expand Up @@ -215,14 +215,14 @@ public function sendBookingInformationEmail(Booking $booking, AppointmentConfig
try {
$failed = $this->mailer->send($message);
if (count($failed) > 0) {
$this->logger->warning('Mail delivery failed for some recipients.');
$this->logger->warning('Mail delivery failed for some recipients.', ['app' => 'calendar-appointments']);
foreach ($failed as $fail) {
$this->logger->debug('Failed to deliver email to ' . $fail);
$this->logger->debug('Failed to deliver email to ' . $fail, ['app' => 'calendar-appointments']);
}
throw new ServiceException('Could not send mail for recipient(s) ' . implode(', ', $failed));
}
} catch (Exception $ex) {
$this->logger->error($ex->getMessage(), ['exception' => $ex]);
$this->logger->error($ex->getMessage(), ['exception' => $ex, 'app' => 'calendar-appointments']);
throw new ServiceException('Could not send mail: ' . $ex->getMessage(), $ex->getCode(), $ex);
}
}
Expand Down Expand Up @@ -320,14 +320,14 @@ public function sendOrganizerBookingInformationEmail(Booking $booking, Appointme
try {
$failed = $this->mailer->send($message);
if (count($failed) > 0) {
$this->logger->warning('Mail delivery failed for some recipients.');
$this->logger->warning('Mail delivery failed for some recipients.', ['app' => 'calendar-appointments']);
foreach ($failed as $fail) {
$this->logger->debug('Failed to deliver email to ' . $fail);
$this->logger->debug('Failed to deliver email to ' . $fail, ['app' => 'calendar-appointments']);
}
throw new ServiceException('Could not send mail for recipient(s) ' . implode(', ', $failed));
}
} catch (Exception $ex) {
$this->logger->error('Could not send appointment organizer email: ' . $ex->getMessage(), ['exception' => $ex]);
$this->logger->error('Could not send appointment organizer email: ' . $ex->getMessage(), ['exception' => $ex, 'app' => 'calendar-appointments']);
throw new ServiceException('Could not send mail: ' . $ex->getMessage(), $ex->getCode(), $ex);
}
}
Expand Down
14 changes: 13 additions & 1 deletion lib/Service/Appointments/SlotExtrapolator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,28 @@
namespace OCA\Calendar\Service\Appointments;

use OCA\Calendar\Db\AppointmentConfig;
use Psr\Log\LoggerInterface;

class SlotExtrapolator {

public function __construct(private LoggerInterface $logger) {

}
/**
* @param AppointmentConfig $config
* @param Interval[] $availabilityIntervals
* @param int $to
*
* @return Interval[]
*/
public function extrapolate(AppointmentConfig $config,
array $availabilityIntervals): array {
$this->logger->debug('Intervals before extrapolating:' . count($availabilityIntervals), ['app' => 'calendar-appointments']);
if(empty($availabilityIntervals)) {
return [];
}
foreach ($availabilityIntervals as $availabilityInterval) {
$this->logger->debug('Interval start: ' . $availabilityInterval->getStart() . ', interval end: ' . $availabilityInterval->getEnd(), ['app' => 'calendar-appointments']);
}
$increment = $config->getIncrement();
$length = $config->getLength();
$slots = [];
Expand All @@ -50,6 +61,7 @@ public function extrapolate(AppointmentConfig $config,
}
}

$this->logger->debug('Slots after extrapolating:' . count($slots), ['app' => 'calendar-appointments']);
return $slots;
}
}
13 changes: 6 additions & 7 deletions tests/php/unit/Service/Appointments/AvailabilityGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,12 @@
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Calendar\ICalendarQuery;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;

class AvailabilityGeneratorTest extends TestCase {
/** @var ITimeFactory|MockObject */
private $timeFactory;

/** @var AvailabilityGenerator */
private $generator;

private ITimeFactory|MockObject $timeFactory;
private MockObject|LoggerInterface $logger;
private AvailabilityGenerator $generator;
protected function setUp(): void {
parent::setUp();

Expand All @@ -50,9 +48,10 @@ protected function setUp(): void {
}

$this->timeFactory = $this->createMock(ITimeFactory::class);

$this->logger = $this->createMock(LoggerInterface::class);
$this->generator = new AvailabilityGenerator(
$this->timeFactory,
$this->logger,
);
}

Expand Down
11 changes: 6 additions & 5 deletions tests/php/unit/Service/Appointments/DailyLimitFilterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@
use OCP\Calendar\ICalendarQuery;
use OCP\Calendar\IManager;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;

/**
* @requires OCP\Calendar\ICalendarQuery::newQuery
*/
class DailyLimitFilterTest extends TestCase {
/** @var IManager|MockObject */
private $manager;

/** @var DailyLimitFilter */
private $filter;
private IManager|MockObject $manager;
private MockObject|LoggerInterface $logger;
private DailyLimitFilter $filter;

protected function setUp(): void {
parent::setUp();
Expand All @@ -51,9 +50,11 @@ protected function setUp(): void {
}

$this->manager = $this->createMock(IManager::class);
$this->logger = $this->createMock(LoggerInterface::class);

$this->filter = new DailyLimitFilter(
$this->manager,
$this->logger,
);
}

Expand Down
6 changes: 5 additions & 1 deletion tests/php/unit/Service/Appointments/SlotExtrapolatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@
use OCA\Calendar\Service\Appointments\Interval;
use OCA\Calendar\Service\Appointments\SlotExtrapolator;
use OCP\Calendar\ICalendarQuery;
use Psr\Log\LoggerInterface;
use Psr\Log\LoggerInterface\PHPUnit\Framework\MockObject\MockObject;

class SlotExtrapolatorTest extends TestCase {
/** @var SlotExtrapolator */
private $extrapolator;
private MockObject|LoggerInterface $logger;

protected function setUp(): void {
parent::setUp();
Expand All @@ -42,7 +45,8 @@ protected function setUp(): void {
self::markTestIncomplete();
}

$this->extrapolator = new SlotExtrapolator();
$this->logger = $this->createMock(LoggerInterface::class);
$this->extrapolator = new SlotExtrapolator($this->logger);
}

public function testNoAvailability(): void {
Expand Down
Loading