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 6 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)
{
$this->cache->put($event->mutexName(), true, 1440);
Copy link

@alain-lf alain-lf Mar 11, 2017

Choose a reason for hiding this comment

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

1440 = 24 hours.

Depending on your workload, if one of your servers goes down, you might not want to wait a full day for the key to expire and resume schedule.

}

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->put($this->mutexName(), true, 1440);
$this->overlappingStrategy->prevent($this);
}

$this->runInBackground
Expand Down Expand Up @@ -516,9 +515,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 void
*/
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
73 changes: 73 additions & 0 deletions tests/Console/Scheduling/CacheOverlappingStrategyTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

namespace Illuminate\Tests\Console\Scheduling;

use Mockery as m;
use PHPUnit\Framework\TestCase;
use Illuminate\Console\Scheduling\Event;
use Illuminate\Console\Scheduling\CacheOverlappingStrategy;

class CacheOverlappingStrategyTest extends TestCase
{
/**
* @var CacheOverlappingStrategy
*/
protected $cacheOverlappingStrategy;

/**
* @var \Illuminate\Contracts\Cache\Repository
*/
protected $cacheRepository;

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

$this->cacheRepository = m::mock('Illuminate\Contracts\Cache\Repository');
$this->cacheOverlappingStrategy = new CacheOverlappingStrategy($this->cacheRepository);
}

public function testPreventOverlap()
{
$cacheOverlappingStrategy = $this->cacheOverlappingStrategy;

$this->cacheRepository->shouldReceive('put');

$event = new Event($this->cacheOverlappingStrategy, 'command');

$cacheOverlappingStrategy->prevent($event);
}

public function testOverlapsForNonRunningTask()
{
$cacheOverlappingStrategy = $this->cacheOverlappingStrategy;

$this->cacheRepository->shouldReceive('has')->andReturn(false);

$event = new Event($this->cacheOverlappingStrategy, 'command');

$this->assertFalse($cacheOverlappingStrategy->overlaps($event));
}

public function testOverlapsForRunningTask()
{
$cacheOverlappingStrategy = $this->cacheOverlappingStrategy;

$this->cacheRepository->shouldReceive('has')->andReturn(true);

$event = new Event($this->cacheOverlappingStrategy, 'command');

$this->assertTrue($cacheOverlappingStrategy->overlaps($event));
}

public function testResetOverlap()
{
$cacheOverlappingStrategy = $this->cacheOverlappingStrategy;

$this->cacheRepository->shouldReceive('forget');

$event = new Event($this->cacheOverlappingStrategy, 'command');

$cacheOverlappingStrategy->reset($event);
}
}
Loading