Skip to content

Commit

Permalink
notification of share through email moved to background job
Browse files Browse the repository at this point in the history
It is better to move the notification of share using
email to background job. This prevents timeout or long
time wait to notify the user(s).

Signed-off-by: Sujith H <[email protected]>
  • Loading branch information
sharidas committed Jul 31, 2018
1 parent 00abb1d commit a3aae6b
Show file tree
Hide file tree
Showing 5 changed files with 525 additions and 6 deletions.
4 changes: 4 additions & 0 deletions appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
<personal>OCA\Notifications\Panels\Personal\NotificationsPanel</personal>
</settings>

<background-jobs>
<job>OCA\Notifications\Background\MailNotificationSender</job>
</background-jobs>

<dependencies>
<owncloud min-version="10.0.9" max-version="10.1" />
</dependencies>
Expand Down
21 changes: 19 additions & 2 deletions lib/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
namespace OCA\Notifications;


use OCP\BackgroundJob\IJobList;
use OCP\Notification\IApp;
use OCP\Notification\INotification;
use OCA\Notifications\Mailer\NotificationMailerAdapter;
Expand All @@ -33,9 +34,13 @@ class App implements IApp {
/** @var NotificationMailerAdapter */
protected $mailerAdapter;

public function __construct(Handler $handler, NotificationMailerAdapter $mailerAdapter) {
/** @var IJobList */
protected $jobList;

public function __construct(Handler $handler, NotificationMailerAdapter $mailerAdapter, IJobList $jobList) {
$this->handler = $handler;
$this->mailerAdapter = $mailerAdapter;
$this->jobList = $jobList;
}

/**
Expand All @@ -46,7 +51,19 @@ public function __construct(Handler $handler, NotificationMailerAdapter $mailerA
*/
public function notify(INotification $notification) {
$this->handler->add($notification);
$this->mailerAdapter->sendMail($notification);
}

/**
* Notification of a share to user(s) by email is achieved by background job
* @param string $shareFullId
* @return null
*/
public function emailNotify($shareFullId) {
$this->jobList->add('OCA\Notifications\BackgroundJob\MailNotificationSender',
[
'shareFullId' => $shareFullId,
'webroot' => \OC::$WEBROOT //This is required to get the url link correctly
]);
}

/**
Expand Down
192 changes: 192 additions & 0 deletions lib/BackgroundJob/MailNotificationSender.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
<?php
/**
* @author Sujith Haridasan <[email protected]>
*
* @copyright Copyright (c) 2018, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace OCA\Notifications\BackgroundJob;

use OC\BackgroundJob\Job;
use OC\User\Manager;
use OCA\Notifications\Configuration\OptionsStorage;
use OCA\Notifications\Mailer\NotificationMailer;
use OCA\Notifications\Mailer\NotificationMailerAdapter;
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\Mail\IMailer;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager;
use OCP\Notification\IManager as NotificationManager;

/**
* Class MailNotificationSender
*
* @package OCA\Notifications\BackgroundJob
*/
class MailNotificationSender extends Job {

/** @var IManager */
private $shareManager;

/** @var \OC\Group\Manager|IGroupManager */
private $groupManager;

/** @var Manager */
private $userManager;

/** @var NotificationManager */
private $notificationManager;

/** @var IConfig */
private $config;

/** @var IMailer */
private $mailer;

/** @var NotificationMailerAdapter */
private $mailerAdapter;

/** @var IURLGenerator */
private $urlGenerator;

/** @var IRequest */
private $request;

/** @var ILogger */
private $logger;

public function __construct(IManager $shareManager = null,
IGroupManager $groupManager = null,
Manager $userManager = null,
NotificationManager $notificationManager = null,
NotificationMailerAdapter $mailerAdapter = null,
IMailer $mailer = null,
IConfig $config = null,
IURLGenerator $urlGenerator = null,
IRequest $request = null,
ILogger $logger = null) {
$this->shareManager = $shareManager ? $shareManager : \OC::$server->getShareManager();
$this->groupManager = $groupManager ? $groupManager : \OC::$server->getGroupManager();
$this->userManager = $userManager ? $userManager : \OC::$server->getUserManager();
$this->notificationManager = $notificationManager ? $notificationManager : \OC::$server->getNotificationManager();
$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
$this->request = $request ? $request : \OC::$server->getRequest();
$this->logger = $logger ? $logger : \OC::$server->getLogger();
$this->mailer = $mailer ? $mailer : \OC::$server->getMailer();
$this->config = $config ? $config : \OC::$server->getConfig();

$optionsStorage = new OptionsStorage($this->config);
$notificationMailer = new NotificationMailer($this->notificationManager, $this->mailer, $optionsStorage);

$this->mailerAdapter = $mailerAdapter ? $mailerAdapter :
new NotificationMailerAdapter($notificationMailer, $this->userManager, $this->logger, $this->urlGenerator);
}

/**
* @param $notificationURL
* @return string
*/
public function fixURL($notificationURL) {
$url = $this->request->getServerProtocol() . '://' . $this->request->getServerHost();
$partURL = \explode($url, $notificationURL)[1];
$webRoot = $this->getArgument()['webroot'];
if (\strpos($partURL, $webRoot) === false) {
$notificationURL = $url . $this->getArgument()['webroot'] . $partURL;
}
return $notificationURL;
}

/**
* @param $shareFullId
*/
public function sendNotify($shareFullId) {
$notificationList = [];
$maxCountSendNotification = 100;

//Check if its an internal share or not
try {
$share = $this->shareManager->getShareById($shareFullId);
} catch (ShareNotFound $e) {
return null;
}

$users = [];
if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
//Notify all the group members
$group = $this->groupManager->get($share->getSharedWith());
$users = $group->getUsers();
} elseif ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER) {
$users[] = $this->userManager->get($share->getSharedWith());
}

foreach ($users as $user) {
if ($user->getEMailAddress() === null) {
\OC::$server->getLogger()->warning("Mail notification can not be sent to " . $user->getUID() . " for share " . $share->getName() . " as email for user is not set");
continue;
}
$notification = $this->notificationManager->createNotification();
$notification->setApp('files_sharing')
->setUser($user->getUID())
->setDateTime(new \DateTime())
->setObject('local_share', $shareFullId);

$fileLink = $this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileId' => $share->getNode()->getId()]);

$fileLink = $this->fixURL($fileLink);
$notification->setLink($fileLink);

$notification->setSubject('local_share', [$share->getShareOwner(), $share->getSharedBy(), $share->getNode()->getName()]);
$notification->setMessage('local_share', [$share->getShareOwner(), $share->getSharedBy(), $share->getNode()->getName()]);

//Adding action because sendMail requires an action for the notificaiton
$acceptAction = $notification->createAction();
$acceptAction->setLabel('email');
$acceptAction->setLink($fileLink, 'POST');
$notification->addAction($acceptAction);

if (\count($notificationList) < $maxCountSendNotification) {
$notificationList[] = $notification;
} else {
foreach ($notificationList as $notificationDispatch) {
$this->mailerAdapter->sendMail($notificationDispatch);
}

//Once users in notificationList recieve notification reset the list
$notificationList = [$notification];
}
}

if (\count($notificationList) < $maxCountSendNotification) {
foreach ($notificationList as $notificationDispatch) {
$this->mailerAdapter->sendMail($notificationDispatch);
}
}
}

protected function run($argument) {
if (($this->getArgument()['shareFullId'] === null) || ($this->getArgument()['webroot'] === null)) {
$this->logger->debug(__METHOD__ . " To execute this background job shareFullId and webroot are required. Aborting the job");
} else {
$this->sendNotify($this->getArgument()['shareFullId']);
}
\OC::$server->getJobList()->removeById($this->getId());
}
}
18 changes: 14 additions & 4 deletions tests/Unit/AppTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

namespace OCA\Notifications\Tests\Unit;

use OCP\BackgroundJob\IJobList;
use OCP\Notification\INotification;
use OCA\Notifications\App;
use OCA\Notifications\Handler;
Expand All @@ -40,6 +41,8 @@ class AppTest extends TestCase {
/** @var \OCA\Notifications\App */
protected $app;

protected $jobList;

protected function setUp() {
parent::setUp();

Expand All @@ -55,23 +58,30 @@ protected function setUp() {
->disableOriginalConstructor()
->getMock();

$this->jobList = $this->createMock(IJobList::class);

$this->app = new App(
$this->handler,
$this->mailerAdapter
$this->mailerAdapter,
$this->jobList
);
}

public function testNotify() {
$this->handler->expects($this->once())
->method('add')
->with($this->notification);
$this->mailerAdapter->expects($this->once())
->method('sendMail')
->with($this->notification);

$this->app->notify($this->notification);
}

public function testEmailNotify() {
$this->jobList->expects($this->once())
->method('add')
->with('OCA\Notifications\BackgroundJob\MailNotificationSender', ['shareFullId' => 'ocinternal:12', 'webroot' => \OC::$WEBROOT]);
$this->app->emailNotify('ocinternal:12');
}

public function testGetCount() {
$this->handler->expects($this->once())
->method('count')
Expand Down
Loading

0 comments on commit a3aae6b

Please sign in to comment.