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.

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

Send queued webhooks in reversed chronological order #4627

1 change: 1 addition & 0 deletions app/bundles/WebhookBundle/Config/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,5 +110,6 @@
'webhook_disable_limit' => 100, // How many times the webhook response can fail until the webhook will be unpublished
'webhook_timeout' => 15, // How long the CURL request can wait for response before Mautic hangs up. In seconds
'queue_mode' => 'immediate_process', // Trigger the webhook immediately or queue it for faster response times
'events_orderby_dir' => \Doctrine\Common\Collections\Criteria::ASC, // Order the queued events chronologically or the other way around
],
];
50 changes: 45 additions & 5 deletions app/bundles/WebhookBundle/Entity/Webhook.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
namespace Mautic\WebhookBundle\Entity;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Criteria;
use Doctrine\DBAL\Types\Type;
use Doctrine\ORM\Mapping as ORM;
use Mautic\ApiBundle\Serializer\Driver\ApiMetadataDriver;
use Mautic\CategoryBundle\Entity\Category;
Expand Down Expand Up @@ -84,6 +86,14 @@ class Webhook extends FormEntity
*/
private $triggers = [];

/**
* ASC or DESC order for fetching order of the events when queue mode is on.
* Null means use the global default.
*
* @var string
*/
private $eventsOrderbyDir;

/*
* Constructor
*/
Expand Down Expand Up @@ -116,7 +126,6 @@ public static function loadMetadata(ORM\ClassMetadata $metadata)
->cascadeDetach()
->build();

// 1:M for queues
$builder->createOneToMany('queues', 'WebhookQueue')
->mappedBy('webhook')
->fetchExtraLazy()
Expand All @@ -125,20 +134,20 @@ public static function loadMetadata(ORM\ClassMetadata $metadata)
->cascadeDetach()
->build();

// 1:M for logs
$builder->createOneToMany('logs', 'Log')->setOrderBy(['dateAdded' => 'DESC'])
$builder->createOneToMany('logs', 'Log')->setOrderBy(['dateAdded' => Criteria::DESC])
->fetchExtraLazy()
->mappedBy('webhook')
->cascadePersist()
->cascadeMerge()
->cascadeDetach()
->build();

// status code
$builder->createField('webhookUrl', 'string')
$builder->createField('webhookUrl', Type::STRING)
->columnName('webhook_url')
->length(255)
->build();

$builder->addNullableField('eventsOrderbyDir', Type::STRING, 'events_orderby_dir');
}

/**
Expand All @@ -155,6 +164,7 @@ public static function loadApiMetadata(ApiMetadataDriver $metadata)
'name',
'description',
'webhookUrl',
'eventsOrderbyDir',
'category',
'triggers',
]
Expand Down Expand Up @@ -193,6 +203,17 @@ public static function loadValidatorMetadata(ClassMetadata $metadata)
]
)
);

$metadata->addPropertyConstraint(
'eventsOrderbyDir',
new Assert\Choice(
[
null,
Criteria::ASC,
Criteria::DESC,
]
)
);
}

/**
Expand Down Expand Up @@ -422,6 +443,25 @@ public function removeEvent(Event $event)
return $this;
}

/**
* @param string $eventsOrderbyDir
*/
public function setEventsOrderbyDir($eventsOrderbyDir)
{
$this->isChanged('eventsOrderbyDir', $eventsOrderbyDir);
$this->eventsOrderbyDir = $eventsOrderbyDir;

return $this;
}

/**
* @return string
*/
public function getEventsOrderbyDir()
{
return $this->eventsOrderbyDir;
}

/**
* @return ArrayCollection
*/
Expand Down
14 changes: 14 additions & 0 deletions app/bundles/WebhookBundle/Form/Type/ConfigType.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Mautic\WebhookBundle\Form\Type;

use Doctrine\Common\Collections\Criteria;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Validator\Constraints\NotBlank;
Expand Down Expand Up @@ -45,6 +46,19 @@ public function buildForm(FormBuilderInterface $builder, array $options)
),
],
]);

$builder->add('events_orderby_dir', 'choice', [
'choices' => [
Criteria::ASC => 'mautic.webhook.config.event.orderby.chronological',
Criteria::DESC => 'mautic.webhook.config.event.orderby.reverse.chronological',
],
'label' => 'mautic.webhook.config.event.orderby',
'attr' => [
'class' => 'form-control',
'tooltip' => 'mautic.webhook.config.event.orderby.tooltip',
],
'required' => false,
]);
}

/**
Expand Down
16 changes: 16 additions & 0 deletions app/bundles/WebhookBundle/Form/Type/WebhookType.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Mautic\WebhookBundle\Form\Type;

use Doctrine\Common\Collections\Criteria;
use Mautic\CoreBundle\Form\EventListener\CleanFormSubscriber;
use Mautic\WebhookBundle\Form\DataTransformer\EventsToArrayTransformer;
use Symfony\Component\Form\AbstractType;
Expand Down Expand Up @@ -125,6 +126,21 @@ public function buildForm(FormBuilderInterface $builder, array $options)
);

$builder->add('isPublished', 'yesno_button_group');

$builder->add('eventsOrderbyDir', 'choice', [
'choices' => [
'' => 'mautic.core.form.default',
Criteria::ASC => 'mautic.webhook.config.event.orderby.chronological',
Criteria::DESC => 'mautic.webhook.config.event.orderby.reverse.chronological',
],
'label' => 'mautic.webhook.config.event.orderby',
'attr' => [
'class' => 'form-control',
'tooltip' => 'mautic.webhook.config.event.orderby.tooltip',
],
'empty_value' => '',
'required' => false,
]);
}

/**
Expand Down
35 changes: 32 additions & 3 deletions app/bundles/WebhookBundle/Model/WebhookModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Mautic\WebhookBundle\Model;

use Doctrine\Common\Collections\Criteria;
use JMS\Serializer\SerializationContext;
use JMS\Serializer\Serializer;
use Joomla\Http\Http;
Expand Down Expand Up @@ -103,6 +104,14 @@ class WebhookModel extends FormModel
*/
protected $notificationModel;

/**
* Queued events default order by dir
* Possible values: ['ASC', 'DESC'].
*
* @var string
*/
protected $eventsOrderByDir;

/**
* WebhookModel constructor.
*
Expand All @@ -121,6 +130,7 @@ public function __construct(
$this->webhookTimeout = (int) $coreParametersHelper->getParameter('webhook_timeout', 15);
$this->logMax = (int) $coreParametersHelper->getParameter('webhook_log_max', 1000);
$this->queueMode = $coreParametersHelper->getParameter('queue_mode');
$this->eventsOrderByDir = $coreParametersHelper->getParameter('events_orderby_dir', Criteria::ASC);
$this->serializer = $serializer;
$this->notificationModel = $notificationModel;
}
Expand Down Expand Up @@ -550,7 +560,7 @@ public function getWebhookPayload(Webhook $webhook)
}

/**
* Get the queues and order by date so we get events in chronological order.
* Get the queues and order by date so we get events.
*
* @param Webhook $webhook
*
Expand All @@ -566,11 +576,12 @@ public function getWebhookQueues(Webhook $webhook)
'iterator_mode' => true,
'start' => $this->webhookStart,
'limit' => $this->webhookLimit,
'orderBy' => 'e.dateAdded', // e is the default prefix unless you define getTableAlias in your repo class,
'orderBy' => $queueRepo->getTableAlias().'.dateAdded',
'orderByDir' => $this->getEventsOrderbyDir($webhook),
'filter' => [
'force' => [
[
'column' => 'IDENTITY(e.webhook)',
'column' => 'IDENTITY('.$queueRepo->getTableAlias().'.webhook)',
'expr' => 'eq',
'value' => $webhook->getId(),
],
Expand All @@ -582,6 +593,24 @@ public function getWebhookQueues(Webhook $webhook)
return $queues;
}

/**
* Returns either Webhook's orderbyDir or the value from configuration as default.
*
* @param Webhook|null $webhook
*
* @return string
*/
public function getEventsOrderbyDir(Webhook $webhook = null)
{
// Try to get the value from Webhook
if ($webhook && $orderByDir = $webhook->getEventsOrderbyDir()) {
return $orderByDir;
}

// Use the global config value if it's not set in the Webhook
return $this->eventsOrderByDir;
}

/**
* {@inheritdoc}
*
Expand Down
67 changes: 67 additions & 0 deletions app/bundles/WebhookBundle/Tests/Model/WebhookModelTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

/*
* @copyright 2016 Mautic Contributors. All rights reserved
* @author Mautic, Inc.
*
* @link https://mautic.org
*
* @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
*/

namespace Mautic\WebhookBundle\Tests\Model;

use JMS\Serializer\Serializer;
use Mautic\CoreBundle\Helper\CoreParametersHelper;
use Mautic\CoreBundle\Model\NotificationModel;
use Mautic\WebhookBundle\Entity\Webhook;
use Mautic\WebhookBundle\Model\WebhookModel;

class WebhookModelTest extends \PHPUnit_Framework_TestCase
{
public function testGetEventsOrderbyDirWhenSetInWebhook()
{
$webhook = (new Webhook())->setEventsOrderbyDir('DESC');
$model = $this->initModel('ASC');
$orderbyDir = $model->getEventsOrderbyDir($webhook);

$this->assertEquals('DESC', $orderbyDir);
}

public function testGetEventsOrderbyDirWhenNotSetInWebhook()
{
$model = $this->initModel('DESC');
$orderbyDir = $model->getEventsOrderbyDir();

$this->assertEquals('DESC', $orderbyDir);
}

public function testGetEventsOrderbyDirWhenWebhookNotProvided()
{
$model = $this->initModel('DESC');
$orderbyDir = $model->getEventsOrderbyDir();

$this->assertEquals('DESC', $orderbyDir);
}

protected function initModel($config = 'someValue')
{
$parametersHelper = $this->getMockBuilder(CoreParametersHelper::class)
->disableOriginalConstructor()
->getMock();

$parametersHelper->expects($this->any())
->method('getParameter')
->will($this->returnValue($config));

$serializer = $this->getMockBuilder(Serializer::class)
->disableOriginalConstructor()
->getMock();

$notificationModel = $this->getMockBuilder(NotificationModel::class)
->disableOriginalConstructor()
->getMock();

return new WebhookModel($parametersHelper, $serializer, $notificationModel);
}
}
4 changes: 4 additions & 0 deletions app/bundles/WebhookBundle/Translations/en_US/messages.ini
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ mautic.webhook.runtime="Request Runtime"
mautic.webhook.killed="Webhook was automatically unpublished, because the response has errored out %limit% times in a row."
mautic.notification.permissions.header="Notification Permissions"
mautic.notification.permissions.notifications="Notifications - User has access to"
mautic.webhook.config.event.orderby="Order of the queued events"
mautic.webhook.config.event.orderby.tooltip="One webhook request can contain several events if queue is used. In what order do you want them to be?"
mautic.webhook.config.event.orderby.chronological="Chronological"
mautic.webhook.config.event.orderby.reverse.chronological="Reverse Chronological"
mautic.webhook.event.sendwebhook="Send a webhook"
mautic.webhook.event.sendwebhook_desc="Send a webhook (only for experienced users)"
mautic.campaign.campaign.sendwebhook="Send a webhook"
Expand Down
1 change: 1 addition & 0 deletions app/bundles/WebhookBundle/Views/Webhook/form.html.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
<div class="col-md-3 bg-white height-auto bdr-l">
<div class="pr-lg pl-lg pt-md pb-md">
<?php echo $view['form']->row($form['category']); ?>
<?php echo $view['form']->row($form['eventsOrderbyDir']); ?>
<?php echo $view['form']->row($form['isPublished']); ?>
</div>
</div>
Expand Down
39 changes: 39 additions & 0 deletions app/migrations/Version20170810120452.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

/*
* @package Mautic
* @copyright 2017 Mautic Contributors. All rights reserved.
* @author Mautic
* @link http://mautic.org
* @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
*/

namespace Mautic\Migrations;

use Doctrine\DBAL\Migrations\SkipMigrationException;
use Doctrine\DBAL\Schema\Schema;
use Mautic\CoreBundle\Doctrine\AbstractMauticMigration;

class Version20170810120452 extends AbstractMauticMigration
{
/**
* @param Schema $schema
*
* @throws SkipMigrationException
* @throws \Doctrine\DBAL\Schema\SchemaException
*/
public function preUp(Schema $schema)
{
if ($schema->getTable($this->prefix.'webhooks')->hasColumn('events_orderby_dir')) {
throw new SkipMigrationException('Schema includes this migration');
}
}

/**
* @param Schema $schema
*/
public function up(Schema $schema)
{
$this->addSql('ALTER TABLE '.$this->prefix.'webhooks ADD events_orderby_dir VARCHAR(255) DEFAULT NULL');
}
}