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

[5.4] add OverlappingStrategy for Schedule/Event #18295

Merged
merged 8 commits into from
Mar 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions src/Illuminate/Console/Scheduling/CacheOverlappingStrategy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

namespace Illuminate\Console\Scheduling;

use Illuminate\Contracts\Cache\Repository as Cache;

class CacheOverlappingStrategy implements OverlappingStrategy
{
/**
* @var \Illuminate\Contracts\Cache\Repository
*/
private $cache;

public function __construct(Cache $cache)
{
$this->cache = $cache;
}

public function prevent(Event $event)
{
return $this->cache->add($event->mutexName(), true, 1440);
}

public function overlaps(Event $event)
{
return $this->cache->has($event->mutexName());
}

public function reset(Event $event)
{
$this->cache->forget($event->mutexName());
}
}
13 changes: 6 additions & 7 deletions src/Illuminate/Console/Scheduling/CallbackEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use LogicException;
use InvalidArgumentException;
use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Cache\Repository as Cache;

class CallbackEvent extends Event
{
Expand All @@ -26,22 +25,22 @@ class CallbackEvent extends Event
/**
* Create a new event instance.
*
* @param \Illuminate\Contracts\Cache\Repository $cache
* @param OverlappingStrategy $overlappingStrategy
* @param string $callback
* @param array $parameters
* @return void
*
* @throws \InvalidArgumentException
*/
public function __construct(Cache $cache, $callback, array $parameters = [])
public function __construct(OverlappingStrategy $overlappingStrategy, $callback, array $parameters = [])
{
if (! is_string($callback) && ! is_callable($callback)) {
throw new InvalidArgumentException(
'Invalid scheduled callback event. Must be a string or callable.'
);
}

$this->cache = $cache;
$this->overlappingStrategy = $overlappingStrategy;
$this->callback = $callback;
$this->parameters = $parameters;
}
Expand All @@ -57,7 +56,7 @@ public function __construct(Cache $cache, $callback, array $parameters = [])
public function run(Container $container)
{
if ($this->description) {
$this->cache->put($this->mutexName(), true, 1440);
$this->overlappingStrategy->prevent($this);
}

try {
Expand All @@ -79,7 +78,7 @@ public function run(Container $container)
protected function removeMutex()
{
if ($this->description) {
$this->cache->forget($this->mutexName());
$this->overlappingStrategy->reset($this);
}
}

Expand All @@ -99,7 +98,7 @@ public function withoutOverlapping()
}

return $this->skip(function () {
return $this->cache->has($this->mutexName());
return $this->overlappingStrategy->overlaps($this);
});
}

Expand Down
19 changes: 9 additions & 10 deletions src/Illuminate/Console/Scheduling/Event.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,17 @@
use Symfony\Component\Process\Process;
use Illuminate\Support\Traits\Macroable;
use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Cache\Repository as Cache;

class Event
{
use Macroable, ManagesFrequencies;

/**
* The cache store implementation.
* The overlapping strategy implementation.
*
* @var \Illuminate\Contracts\Cache\Repository
* @var OverlappingStrategy
*/
protected $cache;
protected $overlappingStrategy;

/**
* The command string.
Expand Down Expand Up @@ -131,13 +130,13 @@ class Event
/**
* Create a new event instance.
*
* @param \Illuminate\Contracts\Cache\Repository $cache
* @param OverlappingStrategy $overlappingStrategy
* @param string $command
* @return void
*/
public function __construct(Cache $cache, $command)
public function __construct(OverlappingStrategy $overlappingStrategy, $command)
{
$this->cache = $cache;
$this->overlappingStrategy = $overlappingStrategy;
$this->command = $command;
$this->output = $this->getDefaultOutput();
}
Expand All @@ -161,7 +160,7 @@ public function getDefaultOutput()
public function run(Container $container)
{
if ($this->withoutOverlapping &&
! $this->cache->add($this->mutexName(), true, 1440)) {
! $this->overlappingStrategy->prevent($this)) {
return;
}

Expand Down Expand Up @@ -517,9 +516,9 @@ public function withoutOverlapping()
$this->withoutOverlapping = true;

return $this->then(function () {
$this->cache->forget($this->mutexName());
$this->overlappingStrategy->reset($this);
})->skip(function () {
return $this->cache->has($this->mutexName());
return $this->overlappingStrategy->overlaps($this);
});
}

Expand Down
30 changes: 30 additions & 0 deletions src/Illuminate/Console/Scheduling/OverlappingStrategy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

namespace Illuminate\Console\Scheduling;

interface OverlappingStrategy
{
/**
* prevents overlapping for the given event.
*
* @param Event $event
* @return bool true if the prevent operation was successful
*/
public function prevent(Event $event);

/**
* checks if the given event's command is already running.
*
* @param Event $event
* @return bool
*/
public function overlaps(Event $event);

/**
* resets the overlapping strategy for the given event.
*
* @param Event $event
* @return void
*/
public function reset(Event $event);
}
22 changes: 13 additions & 9 deletions src/Illuminate/Console/Scheduling/Schedule.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@
use Illuminate\Console\Application;
use Illuminate\Container\Container;
use Symfony\Component\Process\ProcessUtils;
use Illuminate\Contracts\Cache\Repository as Cache;

class Schedule
{
/**
* The cache store implementation.
* The overlapping strategy implementation.
*
* @var \Illuminate\Contracts\Cache\Repository
* @var OverlappingStrategy
*/
protected $cache;
protected $overlappingStrategy;

/**
* All of the events on the schedule.
Expand All @@ -26,12 +25,17 @@ class Schedule
/**
* Create a new event instance.
*
* @param \Illuminate\Contracts\Cache\Repository $cache
* @return void
*/
public function __construct(Cache $cache)
public function __construct()
{
$this->cache = $cache;
$container = Container::getInstance();

if (! $container->bound(OverlappingStrategy::class)) {
$this->overlappingStrategy = $container->make(CacheOverlappingStrategy::class);
} else {
$this->overlappingStrategy = $container->make(OverlappingStrategy::class);
}
}

/**
Expand All @@ -43,7 +47,7 @@ public function __construct(Cache $cache)
*/
public function call($callback, array $parameters = [])
{
$this->events[] = $event = new CallbackEvent($this->cache, $callback, $parameters);
$this->events[] = $event = new CallbackEvent($this->overlappingStrategy, $callback, $parameters);

return $event;
}
Expand Down Expand Up @@ -92,7 +96,7 @@ public function exec($command, array $parameters = [])
$command .= ' '.$this->compileParameters($parameters);
}

$this->events[] = $event = new Event($this->cache, $command);
$this->events[] = $event = new Event($this->overlappingStrategy, $command);

return $event;
}
Expand Down
8 changes: 6 additions & 2 deletions tests/Console/ConsoleEventSchedulerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ public function setUp()
{
parent::setUp();

\Illuminate\Container\Container::getInstance()->instance(
'Illuminate\Console\Scheduling\Schedule', $this->schedule = new Schedule(m::mock('Illuminate\Contracts\Cache\Repository'))
$container = \Illuminate\Container\Container::getInstance();

$container->instance('Illuminate\Console\Scheduling\OverlappingStrategy', m::mock('Illuminate\Console\Scheduling\CacheOverlappingStrategy'));

$container->instance(
'Illuminate\Console\Scheduling\Schedule', $this->schedule = new Schedule(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'))
);
}

Expand Down
20 changes: 10 additions & 10 deletions tests/Console/ConsoleScheduledEventTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public function testBasicCronCompilation()
$app->shouldReceive('isDownForMaintenance')->andReturn(false);
$app->shouldReceive('environment')->andReturn('production');

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$event = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertEquals('* * * * * *', $event->getExpression());
$this->assertTrue($event->isDue($app));
$this->assertTrue($event->skip(function () {
Expand All @@ -45,25 +45,25 @@ public function testBasicCronCompilation()
return true;
})->filtersPass($app));

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$event = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertEquals('* * * * * *', $event->getExpression());
$this->assertFalse($event->environments('local')->isDue($app));

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$event = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertEquals('* * * * * *', $event->getExpression());
$this->assertFalse($event->when(function () {
return false;
})->filtersPass($app));

// chained rules should be commutative
$eventA = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$eventB = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$eventA = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$eventB = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertEquals(
$eventA->daily()->hourly()->getExpression(),
$eventB->hourly()->daily()->getExpression());

$eventA = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$eventB = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$eventA = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$eventB = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertEquals(
$eventA->weekdays()->hourly()->getExpression(),
$eventB->hourly()->weekdays()->getExpression());
Expand All @@ -76,11 +76,11 @@ public function testEventIsDueCheck()
$app->shouldReceive('environment')->andReturn('production');
Carbon::setTestNow(Carbon::create(2015, 1, 1, 0, 0, 0));

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$event = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertEquals('* * * * 4 *', $event->thursdays()->getExpression());
$this->assertTrue($event->isDue($app));

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$event = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertEquals('0 19 * * 3 *', $event->wednesdays()->at('19:00')->timezone('EST')->getExpression());
$this->assertTrue($event->isDue($app));
}
Expand All @@ -92,7 +92,7 @@ public function testTimeBetweenChecks()
$app->shouldReceive('environment')->andReturn('production');
Carbon::setTestNow(Carbon::now()->startOfDay()->addHours(9));

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php foo');
$event = new Event(m::mock('Illuminate\Console\Scheduling\OverlappingStrategy'), 'php foo');
$this->assertTrue($event->between('8:00', '10:00')->filtersPass($app));
$this->assertTrue($event->between('9:00', '9:00')->filtersPass($app));
$this->assertFalse($event->between('10:00', '11:00')->filtersPass($app));
Expand Down
Loading