Skip to content

fix(dav): Move DAV app to non deprecated event dispatcher #39190

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

Merged
skjnldsv merged 4 commits into from
Jul 11, 2023
Merged

fix(dav): Move DAV app to non deprecated event dispatcher #39190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f59c740
fix(dav): Move DAV app to non deprecated event dispatcher
nickvergessen Jul 6, 2023
80ee81f
fix(dav): Move listener to actually existing event
nickvergessen Jul 6, 2023
2bf339e
fix(dav): Remove listener for events that never existed anywhere
nickvergessen Jul 6, 2023
e1d4b82
fix(CI): Fix type confusion with Psalm
nickvergessen Jul 7, 2023
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
45 changes: 15 additions & 30 deletions apps/dav/lib/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
*/
namespace OCA\DAV\AppInfo;

use Exception;
use OCA\DAV\BackgroundJob\UpdateCalendarResourcesRoomsBackgroundJob;
use OCA\DAV\CalDAV\Activity\Backend;
use OCA\DAV\CalDAV\AppCalendar\AppCalendarPlugin;
use OCA\DAV\CalDAV\CalendarManager;
Expand Down Expand Up @@ -71,6 +69,7 @@
use OCA\DAV\Events\CardUpdatedEvent;
use OCA\DAV\Events\SubscriptionCreatedEvent;
use OCA\DAV\Events\SubscriptionDeletedEvent;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Federation\Events\TrustedServerRemovedEvent;
use OCA\DAV\HookManager;
use OCA\DAV\Listener\ActivityUpdaterListener;
Expand Down Expand Up @@ -105,7 +104,6 @@
use OCP\IUser;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;
use Throwable;
use function is_null;
Expand Down Expand Up @@ -215,9 +213,8 @@ public function boot(IBootContext $context): void {
}

public function registerHooks(HookManager $hm,
EventDispatcherInterface $dispatcher,
IAppContainer $container,
IServerContainer $serverContainer) {
IEventDispatcher $dispatcher,
IAppContainer $container) {

Check notice

Code scanning / Psalm

DeprecatedInterface

Interface OCP\AppFramework\IAppContainer is marked as deprecated
$hm->setup();

// first time login event setup
Expand All @@ -227,40 +224,28 @@ public function registerHooks(HookManager $hm,
}
});

$dispatcher->addListener('OC\AccountManager::userUpdated', function (GenericEvent $event) use ($container) {
$user = $event->getSubject();
/** @var SyncService $syncService */
$syncService = $container->query(SyncService::class);
$syncService->updateUser($user);
$dispatcher->addListener('OC\AccountManager::userUpdated', function ($event) use ($container) {
Copy link
Member

@ChristophWurst ChristophWurst Jul 6, 2023

Choose a reason for hiding this comment

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

what type is the event now? can it be hinted?

Copy link
Member Author

@nickvergessen nickvergessen Jul 6, 2023

Choose a reason for hiding this comment

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

Still GenericEvent, but then Psalm complains because the event listener does not consume IEvent. Removing silences psalm and makes it work. The actual fix again is creating and only listening to typed events, but time....

if ($event instanceof GenericEvent) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Cannot resolve types for $event - docblock-defined type OCP\EventDispatcher\Event does not contain Symfony\Component\EventDispatcher\GenericEvent
$user = $event->getSubject();
/** @var SyncService $syncService */
$syncService = $container->query(SyncService::class);

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\IContainer::query has been marked as deprecated
$syncService->updateUser($user);
}
});


$dispatcher->addListener('\OCA\DAV\CalDAV\CalDavBackend::updateShares', function (GenericEvent $event) use ($container) {
Copy link
Member Author

@nickvergessen nickvergessen Jul 6, 2023

Choose a reason for hiding this comment

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

4bb31c0#diff-08d9ceb0185b03a66c780ed0fd7eb645b29cf2b1c1e78f6793e46278168d40caL2881

$dispatcher->addListener(CalendarShareUpdatedEvent::class, function (CalendarShareUpdatedEvent $event) use ($container) {
/** @var Backend $backend */
$backend = $container->query(Backend::class);
$backend->onCalendarUpdateShares(
$event->getArgument('calendarData'),
$event->getArgument('shares'),
$event->getArgument('add'),
$event->getArgument('remove')
$event->getCalendarData(),
$event->getOldShares(),
$event->getAdded(),
$event->getRemoved()
);

// Here we should recalculate if reminders should be sent to new or old sharees
});

$eventHandler = function () use ($container, $serverContainer): void {
try {
/** @var UpdateCalendarResourcesRoomsBackgroundJob $job */
$job = $container->query(UpdateCalendarResourcesRoomsBackgroundJob::class);
$job->run([]);
$serverContainer->getJobList()->setLastRun($job);
} catch (Exception $ex) {
$serverContainer->get(LoggerInterface::class)->error($ex->getMessage(), ['exception' => $ex]);
}
};

$dispatcher->addListener('\OCP\Calendar\Resource\ForceRefreshEvent', $eventHandler);
$dispatcher->addListener('\OCP\Calendar\Room\ForceRefreshEvent', $eventHandler);
Comment on lines -262 to -263
Copy link
Member Author

@nickvergessen nickvergessen Jul 6, 2023

Choose a reason for hiding this comment

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

I could not find ForceRefreshEvent as a search term in our github org, apart from this occurance here. I assume there were plans that changed on the way, so I think this is dead code and I just remove it.

Copy link
Member

@tcitworld tcitworld Jul 7, 2023

Choose a reason for hiding this comment

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

Also in #35388 :)

}

public function registerContactsManager(IContactsManager $cm, IAppContainer $container): void {
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/lib/CalDAV/CalDavBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2841,7 +2841,7 @@ public function updateShares(IShareable $shareable, array $add, array $remove):
$calendarId = $shareable->getResourceId();
$calendarRow = $this->getCalendarById($calendarId);
if ($calendarRow === null) {
throw new \RuntimeException('Trying to update shares for innexistant calendar: ' . $calendarId);
throw new \RuntimeException('Trying to update shares for non-existing calendar: ' . $calendarId);
}
$oldShares = $this->getShares($calendarId);

Expand Down