From b302953a633cda40ea95a17dcc8d7886037e2c71 Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Mon, 17 Mar 2025 18:59:14 +0100 Subject: [PATCH 1/2] Refactor relTimeToSeconds to separate function No behavioral changes intended: the negative modifier bug is explicitly included in this refactoring. --- .../Service/ExternalContestSourceService.php | 41 ++++--------------- webapp/src/Utils/Utils.php | 21 ++++++++-- webapp/tests/Unit/Utils/UtilsTest.php | 32 +++++++++++++++ 3 files changed, 57 insertions(+), 37 deletions(-) diff --git a/webapp/src/Service/ExternalContestSourceService.php b/webapp/src/Service/ExternalContestSourceService.php index 8dd43cf696..d6bf05c4df 100644 --- a/webapp/src/Service/ExternalContestSourceService.php +++ b/webapp/src/Service/ExternalContestSourceService.php @@ -690,41 +690,16 @@ protected function validateAndUpdateContest(Event $event, EventData $data): void ->getRepository(Contest::class) ->find($this->getSourceContestId()); - // We need to convert the freeze to a value from the start instead of - // the end so perform some regex magic. - $duration = $data->duration; - $freeze = $data->scoreboardFreezeDuration; - $reltimeRegex = '/^(-)?(\d+):(\d{2}):(\d{2})(?:\.(\d{3}))?$/'; - preg_match($reltimeRegex, $duration, $durationData); - - $durationNegative = ($durationData[1] === '-'); - $fullDuration = $durationNegative ? $duration : ('+' . $duration); + // We need to convert the freeze to a value from the start instead of the end. + $duration = $data->duration; + $durationInSeconds = Utils::relTimeToSeconds($duration); + $fullDuration = Utils::relTime($durationInSeconds, includePlus: true); + $freeze = $data->scoreboardFreezeDuration; if ($freeze !== null) { - preg_match($reltimeRegex, $freeze, $freezeData); - $freezeNegative = ($freezeData[1] === '-'); - $freezeHourModifier = $freezeNegative ? -1 : 1; - $freezeInSeconds = $freezeHourModifier * (int)$freezeData[2] * 3600 - + 60 * (int)$freezeData[3] - + (double)sprintf('%d.%03d', $freezeData[4], $freezeData[5] ?? 0); - $durationHourModifier = $durationNegative ? -1 : 1; - $durationInSeconds = $durationHourModifier * (int)$durationData[2] * 3600 - + 60 * (int)$durationData[3] - + (double)sprintf('%d.%03d', $durationData[4], $durationData[5] ?? 0); - $freezeStartSeconds = $durationInSeconds - $freezeInSeconds; - $freezeHour = floor($freezeStartSeconds / 3600); - $freezeMinutes = floor(($freezeStartSeconds % 3600) / 60); - $freezeSeconds = floor(($freezeStartSeconds % 60) / 60); - $freezeMilliseconds = $freezeStartSeconds - floor($freezeStartSeconds); - - $fullFreeze = sprintf( - '%s%d:%02d:%02d.%03d', - $freezeHour < 0 ? '' : '+', - $freezeHour, - $freezeMinutes, - $freezeSeconds, - $freezeMilliseconds - ); + $freezeInSeconds = Utils::relTimeToSeconds($freeze); + $freezeStartSeconds = $durationInSeconds - $freezeInSeconds; + $fullFreeze = Utils::relTime($freezeStartSeconds, includePlus: true); } else { $fullFreeze = null; } diff --git a/webapp/src/Utils/Utils.php b/webapp/src/Utils/Utils.php index 419c635e77..8f56b49644 100644 --- a/webapp/src/Utils/Utils.php +++ b/webapp/src/Utils/Utils.php @@ -167,6 +167,8 @@ class Utils final public const DAY_IN_SECONDS = 60*60*24; + final public const RELTIME_REGEX = '/^(-)?(\d+):(\d{2}):(\d{2})(?:\.(\d{3}))?$/'; + /** * Returns the milliseconds part of a time stamp truncated at three digits. */ @@ -191,12 +193,12 @@ public static function absTime(mixed $epoch, bool $floored = false): ?string } /** - * Prints a time diff as relative time as (-)?(h)*h:mm:ss(.uuu)? - * (with millis if $floored is false). + * Prints a time diff as relative time as ([-+])?(h)*h:mm:ss(.uuu)? + * (with millis if $floored is false and with + sign only if $includePlus is true). */ - public static function relTime(float $seconds, bool $floored = false): string + public static function relTime(float $seconds, bool $floored = false, bool $includePlus = false): string { - $sign = ($seconds < 0) ? '-' : ''; + $sign = ($seconds < 0) ? '-' : ($includePlus ? '+' : ''); $seconds = abs($seconds); $hours = (int)($seconds / 3600); $minutes = (int)(($seconds - $hours*3600)/60); @@ -206,6 +208,17 @@ public static function relTime(float $seconds, bool $floored = false): string . ($floored ? '' : $millis); } + public static function relTimeToSeconds(string $reltime): float + { + preg_match(self::RELTIME_REGEX, $reltime, $data); + $negative = ($data[1] === '-'); + $modifier = $negative ? -1 : 1; + $seconds = $modifier * (int)$data[2] * 3600 + + (int)$data[3] * 60 + + (float)sprintf('%d.%03d', $data[4], $data[5] ?? 0); + return $seconds; + } + /** * Parse a string as time and return as epoch in float format (with * optional fractional part). The original time string should be in one of diff --git a/webapp/tests/Unit/Utils/UtilsTest.php b/webapp/tests/Unit/Utils/UtilsTest.php index f5ff542aa2..5f35f6b4ea 100644 --- a/webapp/tests/Unit/Utils/UtilsTest.php +++ b/webapp/tests/Unit/Utils/UtilsTest.php @@ -72,6 +72,14 @@ public function testRelTime(): void self::assertEquals('1:18:31.000', Utils::relTime(4711)); } + /** + * Test that the relTime function returns the plus sign + */ + public function testRelTimeWithPlus(): void + { + self::assertEquals('+1:18:31.000', Utils::relTime(4711, includePlus: true)); + } + /** * Test that the relTime function returns the correct data when using a * time with millisecond precision @@ -126,6 +134,30 @@ public function testNegativeRelTimeWithMillisFloored(): void self::assertEquals('-3:25:45', Utils::relTime(-12345.6789, true)); } + /** + * Test the relTimeToSeconds function with basic data. + */ + public function testRelTimeToSeconds(): void + { + self::assertEquals(1*3600 + 2*60 + 3, Utils::relTimeToSeconds('1:02:03')); + } + + /** + * Test the relTimeToSeconds function with millisecond precision. + */ + public function testRelTimeToSecondsWithMillis(): void + { + self::assertEquals(10.123, Utils::relTimeToSeconds('0:00:10.123')); + } + + /** + * Test the relTimeToSeconds function with negative reltime. + */ + public function testRelTimeToSecondsWithNegative(): void + { + self::assertEquals(-(1*3600 + 2*60 + 3), Utils::relTimeToSeconds('-1:02:03')); + } + /** * Test that the toEpochFloat function works with a leap day */ From a624905b3a97055c9fb1d81c5348f0af4d54972f Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Mon, 17 Mar 2025 19:56:49 +0100 Subject: [PATCH 2/2] Fix multiplication of modifier in relTimeToSeconds --- webapp/src/Utils/Utils.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/webapp/src/Utils/Utils.php b/webapp/src/Utils/Utils.php index 8f56b49644..e5d62fac9b 100644 --- a/webapp/src/Utils/Utils.php +++ b/webapp/src/Utils/Utils.php @@ -213,9 +213,10 @@ public static function relTimeToSeconds(string $reltime): float preg_match(self::RELTIME_REGEX, $reltime, $data); $negative = ($data[1] === '-'); $modifier = $negative ? -1 : 1; - $seconds = $modifier * (int)$data[2] * 3600 + $seconds = $modifier * ( + (int)$data[2] * 3600 + (int)$data[3] * 60 - + (float)sprintf('%d.%03d', $data[4], $data[5] ?? 0); + + (float)sprintf('%d.%03d', $data[4], $data[5] ?? 0)); return $seconds; }