From 380512dcbe18aeaa10e7a1d5bbd3eb6138f23cae Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Sep 2024 17:51:52 +0200 Subject: [PATCH] =?UTF-8?q?N=C2=B07845=20Suppress=20risky=20syntax=20with?= =?UTF-8?q?=20DateTime::modify=20(at=20best,=20just=20a=20bad=20example,?= =?UTF-8?q?=20at=20worst,=20just=20a=20result=20that=20will=20vary=20wethe?= =?UTF-8?q?r=20PHP=20>=208.2=20or=20not)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/backgroundprocess.inc.php | 4 +- core/computing.inc.php | 2 +- .../main.itop-sla-computation.php | 2 +- .../core/WeeklyScheduledProcessMockConfig.php | 6 +- .../core/WeeklyScheduledProcessTest.php | 143 +++++++++++++----- webservices/cron.php | 2 +- 6 files changed, 117 insertions(+), 42 deletions(-) diff --git a/core/backgroundprocess.inc.php b/core/backgroundprocess.inc.php index c1c649c57..700b1420e 100644 --- a/core/backgroundprocess.inc.php +++ b/core/backgroundprocess.inc.php @@ -230,7 +230,7 @@ abstract class AbstractWeeklyScheduledProcess implements iScheduledProcess $iFirstDayOfWeek = $aDays[0]; $iDayMove = $oNow->format('N') - $iFirstDayOfWeek; $oRet = clone $oNow; - $oRet->modify('-'.$iDayMove.' days'); + $oRet->modify(-$iDayMove.' days'); $oRet->modify('+1 weeks'); } else @@ -238,7 +238,7 @@ abstract class AbstractWeeklyScheduledProcess implements iScheduledProcess $iNextDayOfWeek = $aDays[$iNextPos]; $iMove = $iNextDayOfWeek - $oNow->format('N'); $oRet = clone $oNow; - $oRet->modify('+'.$iMove.' days'); + $oRet->modify($iMove.' days'); } list($sHours, $sMinutes) = explode(':', $sProcessTime); $oRet->setTime((int)$sHours, (int)$sMinutes); diff --git a/core/computing.inc.php b/core/computing.inc.php index 8df3634d7..f183e737c 100644 --- a/core/computing.inc.php +++ b/core/computing.inc.php @@ -108,7 +108,7 @@ class DefaultWorkingTimeComputer implements iWorkingTimeComputer // Default implementation: 24x7, no holidays: to compute the deadline, just add // the specified duration to the given date/time $oResult = clone $oStartDate; - $oResult->modify('+'.$iDuration.' seconds'); + $oResult->modify($iDuration.' seconds'); if (class_exists('WorkingTimeRecorder')) { WorkingTimeRecorder::SetValues($oStartDate->format('U'), $oResult->format('U'), $iDuration, WorkingTimeRecorder::COMPUTED_END); diff --git a/datamodels/2.x/itop-sla-computation/main.itop-sla-computation.php b/datamodels/2.x/itop-sla-computation/main.itop-sla-computation.php index 4ed2e9079..ef02c08d2 100755 --- a/datamodels/2.x/itop-sla-computation/main.itop-sla-computation.php +++ b/datamodels/2.x/itop-sla-computation/main.itop-sla-computation.php @@ -166,7 +166,7 @@ class SLAComputationAddOnAPI // Default implementation: 24x7, no holidays: to compute the deadline, just add // the specified duration to the given date/time $oResult = clone $oStartDate; - $oResult->modify('+'.$iDuration.' seconds'); + $oResult->modify($iDuration.' seconds'); return $oResult; } diff --git a/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessMockConfig.php b/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessMockConfig.php index bc26199a9..e51f96d45 100644 --- a/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessMockConfig.php +++ b/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessMockConfig.php @@ -9,14 +9,14 @@ */ class WeeklyScheduledProcessMockConfig extends AbstractWeeklyScheduledProcess { - const MODULE_NAME = 'TEST_SCHEDULED_PROCESS'; + const MODULE_NAME = 'itop-zabu-gomeu'; - public function __construct($bEnabledValue, $sTimeValue) + public function __construct($bEnabledValue, $sTimeValue, $sWeekDays) { $this->oConfig = new Config(); $this->oConfig->SetModuleSetting(self::MODULE_NAME, self::MODULE_SETTING_ENABLED, $bEnabledValue); $this->oConfig->SetModuleSetting(self::MODULE_NAME, self::MODULE_SETTING_TIME, $sTimeValue); - $this->oConfig->SetModuleSetting(self::MODULE_NAME, self::MODULE_SETTING_WEEKDAYS, 'monday, tuesday, wednesday, thursday, friday'); + $this->oConfig->SetModuleSetting(self::MODULE_NAME, self::MODULE_SETTING_WEEKDAYS, $sWeekDays); utils::InitTimeZone($this->oConfig); } diff --git a/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessTest.php b/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessTest.php index 18f5d1854..a42cb8337 100644 --- a/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessTest.php +++ b/tests/php-unit-tests/unitary-tests/core/WeeklyScheduledProcessTest.php @@ -14,44 +14,119 @@ class WeeklyScheduledProcessTest extends ItopTestCase $this->RequireOnceUnitTestFile('./WeeklyScheduledProcessMockConfig.php'); } - - /** - * @dataProvider GetNextOccurrenceProvider - * @test - * - * @param boolean $bEnabledValue true if task is enabled - * @param string $sCurrentTime Date string for current time, eg '2020-01-01 23:30' - * @param string $sTimeValue time to run that is set in the config, eg '23:30' - * @param string $sExpectedTime next occurrence that should be returned - * - * @throws \ProcessInvalidConfigException - */ - public function TestGetNextOccurrence($bEnabledValue, $sCurrentTime, $sTimeValue, $sExpectedTime) + public function testShouldPlanForNeverIfDisabled() { - $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig($bEnabledValue, $sTimeValue); - - $oExpectedDateTime = new DateTime($sExpectedTime); - - $this->assertEquals($oExpectedDateTime, $oWeeklyImpl->GetNextOccurrence($sCurrentTime)); + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(false, '22:00', 'monday'); + $this->assertEquals(new DateTime('3000-01-01'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 21:00'), 'Disabled process should be planned for a date far in the future'); } - public function GetNextOccurrenceProvider() + public function testNextOccurrenceShouldGiveTheSameDayWhenBeforeTheLimit() { - return array( - 'Disabled process' => array(false, 'now', null, '3000-01-01'), - 'working day same day, prog noon and current before noon' => array(true, '2020-05-11 11:00', '12:00', '2020-05-11 12:00'), - 'working day same day, prog noon and current is noon' => array(true, '2020-05-11 12:00', '12:00', '2020-05-12 12:00'), - 'working day same day, prog noon and current after noon' => array(true, '2020-05-11 13:00', '12:00', '2020-05-12 12:00'), - 'saturday, prog noon and current before noon' => array(true, '2020-05-09 11:00', '12:00', '2020-05-11 12:00'), - 'saturday, prog noon and current is noon' => array(true, '2020-05-09 12:00', '12:00', '2020-05-11 12:00'), - 'saturday, prog noon and current after noon' => array(true, '2020-05-09 13:00', '12:00', '2020-05-11 12:00'), - 'sunday, prog noon and current before noon' => array(true, '2020-05-10 11:00', '12:00', '2020-05-11 12:00'), - 'sunday, prog noon and current is noon' => array(true, '2020-05-10 12:00', '12:00', '2020-05-11 12:00'), - 'sunday, prog noon and current after noon' => array(true, '2020-05-10 13:00', '12:00', '2020-05-11 12:00'), - 'working day, day before, prog noon and current before midnight' => array(true, '2020-05-11 23:59', '00:00', '2020-05-12 00:00'), - 'working day same day, prog noon and current is midnight' => array(true, '2020-05-12 00:00', '00:00', '2020-05-13 00:00'), - 'working day same day, prog noon and current after midnight' => array(true, '2020-05-12 00:01', '00:00', '2020-05-13 00:00'), - ); + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '22:00', 'monday, tuesday'); + + $this->assertEquals(new DateTime('2020-05-11 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 21:00'), '9 pm should be followed by 10pm'); + $this->assertEquals(new DateTime('2020-05-11 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 21:59'), '9:59 pm should be followed by 10pm'); + $this->assertEquals(new DateTime('2020-05-11 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 21:59:59'), '9:59:59 pm should be followed by 10pm'); + $this->assertEquals(new DateTime('2020-05-11 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 21:59:59.999'), '9:59:59 pm and 999 milliseconds should be followed by 10pm'); + + $this->assertEquals(new DateTime('2020-05-12 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 22:00:00.0'), '10 pm should be followed by 10 pm on the NEXT matching day'); + } + + public function testNextOccurrenceShouldGiveTheNextDayWhateverTheDayOfTheWeek() + { + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '22:00', 'monday, tuesday, wednesday, thursday, friday, saturday, sunday'); + $this->assertEquals(new DateTime('2020-05-12 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 23:00'), 'The occurrence after monday should be tuesday'); + $this->assertEquals(new DateTime('2020-05-13 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-12 23:00'), 'The occurrence after tuesday should be wednesday'); + $this->assertEquals(new DateTime('2020-05-14 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-13 23:00'), 'The occurrence after wednesday should be thursday'); + $this->assertEquals(new DateTime('2020-05-15 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-14 23:00'), 'The occurrence after thursday should be friday'); + $this->assertEquals(new DateTime('2020-05-16 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-15 23:00'), 'The occurrence after friday should be saturday'); + $this->assertEquals(new DateTime('2020-05-17 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-16 23:00'), 'The occurrence after saturday should be sunday'); + $this->assertEquals(new DateTime('2020-05-18 22:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-17 23:00'), 'The occurrence after sunday should be monday'); + } + + public function testNextOccurrenceFindsTheNextWeekWhenOneDayIsConfigured() + { + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'monday'); + $this->assertEquals(new DateTime('2020-05-18 12:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 12:45'), 'The occurrence after monday should be monday of the next week'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'tuesday'); + $this->assertEquals(new DateTime('2020-05-19 12:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-12 12:45'), 'The occurrence after tuesday should be tuesday of the next week'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'wednesday'); + $this->assertEquals(new DateTime('2020-05-20 12:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-13 12:45'), 'The occurrence after wednesday should be wednesday of the next week'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'thursday'); + $this->assertEquals(new DateTime('2020-05-21 12:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-14 12:45'), 'The occurrence after thursday should be thursday of the next week'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'friday'); + $this->assertEquals(new DateTime('2020-05-22 12:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-15 12:45'), 'The occurrence after friday should be friday of the next week'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'saturday'); + $this->assertEquals(new DateTime('2020-05-23 12:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-16 12:45'), 'The occurrence after saturday should be saturday of the next week'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'sunday'); + $this->assertEquals(new DateTime('2020-05-24 12:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-17 12:45'), 'The occurrence after sunday should be sunday of the next week'); + } + + public function testNextOccurrenceShouldCopeWithATaskPlannedAtMidnightOnWeekEnds() + { + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '00:00', 'saturday, sunday, monday'); + $this->assertEquals(new DateTime('2020-05-16 00:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-15 23:59'), 'The occurrence after friday 23:59 should be one second later'); + $this->assertEquals(new DateTime('2020-05-17 00:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-16 00:00'), 'The occurrence after saturday 00:00 should be sunday 00:00'); + $this->assertEquals(new DateTime('2020-05-18 00:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-17 00:00'), 'The occurrence after sunday 00:00 should be monday 00:00'); + $this->assertEquals(new DateTime('2020-05-23 00:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-18 00:00'), 'The occurrence after monday 00:00 should be on next saturday'); + } + + public function testWeekDaysConfigShouldBeCaseInsensitive() + { + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'WEDnESdAY'); + $this->assertEquals([3], $oWeeklyImpl->InterpretWeekDays()); + } + + public function testWeekDaysConfigSpacesShouldBeIgnored() + { + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', ' wednesday ,tuesday'); + $this->assertEquals([2, 3], $oWeeklyImpl->InterpretWeekDays()); + } + + public function testWeekDaysConfigOrderShouldNotMatter() + { + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'sunday, monday, tuesday, thursday'); + $this->assertEquals([1, 2, 4, 7], $oWeeklyImpl->InterpretWeekDays(), 'Days of week are sorted when the configuration is read'); + } + + public function testWeekDaysConfigWithInvalidDayShouldThrowAMeaningfulException() + { + $this->expectException(\ProcessInvalidConfigException::class); + $this->expectExceptionMessage('itop-zabu-gomeu: wrong format for setting \'week_days\' (found \'mercredi\')'); + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12:00', 'monday, tuesday, wednesday, mercredi'); + + $oWeeklyImpl->InterpretWeekDays(); + } + + public function testTimeConfigSecondsShouldBeIgnored() + { + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '22:33:44.123', 'monday'); + $this->assertEquals(new DateTime('2020-05-11 22:33:00'), $oWeeklyImpl->GetNextOccurrence('2020-05-11 21:00')); + } + public function testEmptyTimeConfigShouldThrowAMeaningfulException() + { + $this->expectException(\ProcessInvalidConfigException::class); + $this->expectExceptionMessage('itop-zabu-gomeu: wrong format for setting \'time\' (found \'\')'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '', 'monday'); + + $oWeeklyImpl->GetNextOccurrence(); + } + + public function testBadlyFormattedTimeConfigShouldThrowAMeaningfulException() + { + $this->expectException(\ProcessInvalidConfigException::class); + $this->expectExceptionMessage('itop-zabu-gomeu: wrong format for setting \'time\' (found \'12am\')'); + + $oWeeklyImpl = new \WeeklyScheduledProcessMockConfig(true, '12am', 'monday'); + + $oWeeklyImpl->GetNextOccurrence(); } } diff --git a/webservices/cron.php b/webservices/cron.php index faffbc4d1..f51a46f64 100644 --- a/webservices/cron.php +++ b/webservices/cron.php @@ -168,7 +168,7 @@ function RunTask(BackgroundTask $oTask, $iTimeLimit) // Background processes do repeat periodically $oPlannedStart = clone $oDatePlanned; // Let's schedule from the previous planned date of execution to avoid shift - $oPlannedStart->modify('+'.$oProcess->GetPeriodicity().' seconds'); + $oPlannedStart->modify($oProcess->GetPeriodicity().' seconds'); $oEnd = new DateTime(); while ($oPlannedStart->format('U') < $oEnd->format('U')) {