From fb8fddf6690b5348e8da692b5e19719c421794f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Tue, 15 Sep 2026 11:12:26 +0200 Subject: [PATCH] fix: make snowreport import type-safe and drop wget shell-out --- .../Service/SnowreportImportService.php | 482 +++++++++++++++--- .../typo3conf/ext/ep_products/ext_tables.sql | 2 + 2 files changed, 407 insertions(+), 77 deletions(-) diff --git a/public/typo3conf/ext/ep_products/Classes/Service/SnowreportImportService.php b/public/typo3conf/ext/ep_products/Classes/Service/SnowreportImportService.php index ed82dad2..af0c00c2 100644 --- a/public/typo3conf/ext/ep_products/Classes/Service/SnowreportImportService.php +++ b/public/typo3conf/ext/ep_products/Classes/Service/SnowreportImportService.php @@ -33,7 +33,10 @@ use EP\EpProducts\Domain\Repository\RegionRepository; use EP\EpProducts\Domain\Repository\SnowreportRepository; use EP\EpProducts\Domain\Repository\WebcamRepository; use EP\EpProducts\Utility\SettingsUtility; +use TYPO3\CMS\Core\Database\Connection; use TYPO3\CMS\Core\Database\ConnectionPool; +use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction; +use TYPO3\CMS\Core\Http\RequestFactory; use TYPO3\CMS\Core\SingletonInterface; use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\Persistence\PersistenceManagerInterface; @@ -44,14 +47,32 @@ class SnowreportImportService implements SingletonInterface const XML_FILE_URL = 'https://www.skiresort-service.com/xml-feed/'; /** - * @var array + * Connection and read timeout for the feed download, in seconds. */ - protected static $validSnowreportUids = []; + const HTTP_CONNECT_TIMEOUT = 10; + const HTTP_TIMEOUT = 120; /** * @var array */ - protected static $validWebcamUids = []; + protected $validSnowreportUids = []; + + /** + * @var array + */ + protected $validWebcamUids = []; + + /** + * Existing records keyed by vendor_uid, preloaded once per import to avoid a query per record. + * + * @var array + */ + protected $snowreportIndex = []; + + /** + * @var array + */ + protected $webcamIndex = []; /** * @var \TYPO3\CMS\Extbase\Persistence\Generic\PersistenceManager @@ -91,27 +112,62 @@ class SnowreportImportService implements SingletonInterface $this->regionRepository = $regionRepository; } + const TYPE_INT = 'int'; + const TYPE_STRING = 'string'; + const TYPE_BOOL = 'bool'; + const TYPE_DATE = 'date'; + /** + * Declared schema of the vendor feed: XML element name => target column and its type. + * + * XML carries no type information - every element is a string - so the type of each field is + * declared here rather than guessed from the value. Values are converted strictly according to + * this table; anything unexpected is logged and falls back to the type's empty value instead of + * silently changing type. + * * @var array */ protected static $map = [ - 'region_id' => 'vendor_uid', - 'region_name' => 'region_name', - 'region_offen' => 'opened', - 'gletschergebiet' => 'glacier', - 'schneehoehe_berg' => 'snow_level_mountain', - 'schneehoehe_tal' => 'snow_level_valley', - 'schneequalitaet' => 'snow_quality', - 'letzter_schneefall' => 'last_snowfall', - 'lifte_gesamt' => 'lifts_total', - 'offene_lifte' => 'lifts_opened', - 'letzte_aktualisierung' => 'last_update', - 'hoehemax' => 'height_max', - 'hoehemin' => 'height_min', - 'datum_saisonstart' => 'season_start', - 'datum_saisonende' => 'season_end', + 'region_id' => ['column' => 'vendor_uid', 'type' => self::TYPE_INT], + 'region_name' => ['column' => 'region_name', 'type' => self::TYPE_STRING], + 'region_offen' => ['column' => 'opened', 'type' => self::TYPE_BOOL], + 'gletschergebiet' => ['column' => 'glacier', 'type' => self::TYPE_BOOL], + 'schneehoehe_berg' => ['column' => 'snow_level_mountain', 'type' => self::TYPE_INT], + 'schneehoehe_tal' => ['column' => 'snow_level_valley', 'type' => self::TYPE_INT], + 'schneequalitaet' => ['column' => 'snow_quality', 'type' => self::TYPE_STRING], + 'letzter_schneefall' => ['column' => 'last_snowfall', 'type' => self::TYPE_DATE], + 'lifte_gesamt' => ['column' => 'lifts_total', 'type' => self::TYPE_INT], + 'offene_lifte' => ['column' => 'lifts_opened', 'type' => self::TYPE_INT], + 'letzte_aktualisierung' => ['column' => 'last_update', 'type' => self::TYPE_DATE], + 'hoehemax' => ['column' => 'height_max', 'type' => self::TYPE_INT], + 'hoehemin' => ['column' => 'height_min', 'type' => self::TYPE_INT], + 'datum_saisonstart' => ['column' => 'season_start', 'type' => self::TYPE_DATE], + 'datum_saisonende' => ['column' => 'season_end', 'type' => self::TYPE_DATE], ]; + /** + * Accepted boolean representations, lowercased. + * + * @var array + */ + protected static $booleanMap = [ + 'true' => true, + '1' => true, + 'ja' => true, + 'yes' => true, + 'false' => false, + '0' => false, + 'nein' => false, + 'no' => false, + ]; + + /** + * Distinct conversion problems encountered during the current import. + * + * @var array + */ + protected $conversionWarnings = []; + /** * @return int */ @@ -119,39 +175,41 @@ class SnowreportImportService implements SingletonInterface { $count = 0; $file = GeneralUtility::getFileAbsFileName(self::XML_FILE_PATH); - $xmlContent = file_get_contents($file); - $xmlData = simplexml_load_string($xmlContent); + + if (!is_file($file)) { + $this->writelog(sprintf('Snowreport XML import failed: file "%s" does not exist.', $file)); + + return 0; + } + + try { + $xmlData = $this->parseReportsXml((string) file_get_contents($file)); + } catch (\RuntimeException $e) { + $this->writelog(sprintf('Snowreport XML import failed: %s', $e->getMessage())); + + return 0; + } + + $this->resetImportState(); + $this->buildIndexes(); $snowReportStoragePid = $this->getStoragePid(); foreach ($xmlData->schneemeldung as $reportXml) { $vendorUid = (int) $reportXml->region_id; - $result = $this->snowreportRepository->findByVendorUid($vendorUid); - if ($result->count() === 0) { - $snowreport = new Snowreport(); - $this->snowreportRepository->add($snowreport); - } else { - $snowreport = $result->getFirst(); - } + $snowreport = $this->getSnowreport($vendorUid); $row = []; - foreach (static::$map as $key => $column) + foreach (static::$map as $key => $field) { - $valueRaw = trim($reportXml->{$key}); - - if ($valueRaw === 'true') { - $value = true; - } elseif ($valueRaw === 'false') { - $value = false; - } elseif (preg_match('/^\d{4}\-\d{2}\-\d{2}$/', $valueRaw)) { - $date = new \DateTime($valueRaw); - $value = $date->getTimestamp(); - } else { - $value = (string) $valueRaw; + if (!isset($reportXml->{$key})) { + $this->addConversionWarning(sprintf('Element <%s> missing in feed.', $key)); + $row[$field['column']] = $this->getEmptyValue($field['type']); + continue; } - $row[$column] = $value; + $row[$field['column']] = $this->convertValue((string) $reportXml->{$key}, $field['type'], $key); } $snowreport->setPid($snowReportStoragePid); @@ -175,23 +233,24 @@ class SnowreportImportService implements SingletonInterface $this->importWebcamData($reportXml->webcam, $snowreport); } - if ($snowreport->_isNew()) { - $this->persistenceManager->persistAll(); - } else { + // New objects are already registered via add(); only managed ones need update(). + if (!$snowreport->_isNew()) { $this->snowreportRepository->update($snowreport); } - static::$validSnowreportUids[] = $vendorUid; + $this->validSnowreportUids[] = $vendorUid; $count++; } $this->persistenceManager->persistAll(); - $this->cleanupData('tx_epproducts_domain_model_webcam', static::$validWebcamUids); - $this->cleanupData('tx_epproducts_domain_model_snowreport', static::$validSnowreportUids); + $this->cleanupData('tx_epproducts_domain_model_webcam', $this->validWebcamUids); + $this->cleanupData('tx_epproducts_domain_model_snowreport', $this->validSnowreportUids); + $this->updateWebcamCounters(); - $GLOBALS['BE_USER']->writelog(4, 0, 0, 0, sprintf('Snowreport XML import successful (%d records).', $count), []); + $this->logConversionWarnings(); + $this->writelog(sprintf('Snowreport XML import successful (%d records).', $count)); return $count; } @@ -205,40 +264,52 @@ class SnowreportImportService implements SingletonInterface $snowReportStoragePid = $this->getStoragePid(); foreach ($xml->children() as $item) { - $include = (string) $item->aktuell === 'true'; - $available = (string) $item->verf === 'true'; + $include = $this->convertValue((string) $item->aktuell, self::TYPE_BOOL, 'aktuell'); + $available = $this->convertValue((string) $item->verf, self::TYPE_BOOL, 'verf'); $vendorUid = (int) $item->uid; - $result = $this->webcamRepository->findByVendorUid($vendorUid); - // Skip invalid cams - if ($include === false && $result->count() === 0) { + + $webcam = isset($this->webcamIndex[$vendorUid]) ? $this->webcamIndex[$vendorUid] : null; + + // Skip cams that are neither current nor already known + if (null === $webcam && false === $include) { continue; } + // Add new cam if not in db yet - if ($result->count() === 0) { + if (null === $webcam) { $webcam = new Webcam(); $webcam->setVendorUid($vendorUid); $webcam->setPid($snowReportStoragePid); - $webcam->setUrl((string) $item->url); - $webcam->setName((string) $item->dt); - $webcam->setAvailable($available); $this->webcamRepository->add($webcam); + $this->webcamIndex[$vendorUid] = $webcam; $snowreport->addWebcam($webcam); - } else { - // Update cam in db and set availability - $webcam = $result->getFirst(); - $webcam->setUrl((string) $item->url); - $webcam->setName((string) $item->dt); - $webcam->setAvailable($available && $include); } - static::$validWebcamUids[] = $vendorUid; + + $webcam->setUrl((string) $item->url); + $webcam->setName((string) $item->dt); + $webcam->setAvailable($available && $include); + + $this->validWebcamUids[] = $vendorUid; } } + /** + * Flags records that are no longer contained in the feed as deleted. + * + * @param string $table + * @param array $validUids + * @return int Number of records flagged + */ public function cleanupData($table, array $validUids) { /** @var ConnectionPool $connectionPool */ $connectionPool = GeneralUtility::makeInstance(ConnectionPool::class); $qb = $connectionPool->getQueryBuilderForTable($table); + // Hidden records are still managed by the import, only deleted ones are out of scope. + $qb->getRestrictions() + ->removeAll() + ->add(GeneralUtility::makeInstance(DeletedRestriction::class)) + ; $result = $qb ->select('vendor_uid') ->from($table) @@ -250,36 +321,293 @@ class SnowreportImportService implements SingletonInterface return (int)$row['vendor_uid']; }, $result); - $obsoleteUids = array_diff($currentUids, $validUids); + $obsoleteUids = array_values(array_diff($currentUids, $validUids)); if (0 === count($obsoleteUids)) { - return; + return 0; } $qb = $connectionPool->getQueryBuilderForTable($table); - $qb - ->delete($table) - ->where($qb->expr()->in('vendor_uid', $obsoleteUids)) + + return (int) $qb + ->update($table) + ->set('deleted', 1) + ->set('tstamp', isset($GLOBALS['EXEC_TIME']) ? $GLOBALS['EXEC_TIME'] : time()) + ->where( + $qb->expr()->in( + 'vendor_uid', + $qb->createNamedParameter($obsoleteUids, Connection::PARAM_INT_ARRAY) + ) + ) ->execute() ; } + /** + * Recalculates the inline child counter on the snowreport records. Webcams are flagged as deleted + * with plain SQL, which bypasses the counter Extbase maintains for the relation. + */ + protected function updateWebcamCounters() + { + $connection = GeneralUtility::makeInstance(ConnectionPool::class) + ->getConnectionForTable('tx_epproducts_domain_model_snowreport'); + + $connection->executeStatement( + 'UPDATE tx_epproducts_domain_model_snowreport s SET s.webcams = (' + . ' SELECT COUNT(*) FROM tx_epproducts_domain_model_webcam w' + . ' WHERE w.snowreport = s.uid AND w.deleted = 0' + . ')' + ); + } + + /** + * Downloads the feed and replaces the local file only if the response is a usable XML document. + * The existing file is left untouched on any failure. + * + * @return bool + */ public function downloadReportsXml() { $path = GeneralUtility::getFileAbsFileName(self::XML_FILE_PATH); - $backupPath = $path . '.bak'; - @copy($path, $backupPath); - @unlink($path); + try { + $xmlContent = $this->fetchReportsXml(); + $this->parseReportsXml($xmlContent); + } catch (\Exception $e) { + $this->writelog(sprintf( + 'Snowreport XML download failed (%s). Keeping existing file.', + $e->getMessage() + )); - $url = self::XML_FILE_URL; - exec("wget -O {$path} {$url}", $output, $return); - if (!$return) { - @unlink($backupPath); + return false; + } + + // Write to a temporary file first and move it into place, so the feed is either the old or the + // new document, never a partially written one. + $temporaryPath = $path . '.tmp'; + if (false === file_put_contents($temporaryPath, $xmlContent) || !rename($temporaryPath, $path)) { + @unlink($temporaryPath); + $this->writelog(sprintf('Snowreport XML could not be written to "%s".', $path)); + + return false; + } + + return true; + } + + /** + * @return string + * @throws \RuntimeException + */ + protected function fetchReportsXml() + { + /** @var RequestFactory $requestFactory */ + $requestFactory = GeneralUtility::makeInstance(RequestFactory::class); + + $response = $requestFactory->request(self::XML_FILE_URL, 'GET', [ + 'connect_timeout' => self::HTTP_CONNECT_TIMEOUT, + 'timeout' => self::HTTP_TIMEOUT, + 'headers' => [ + 'Accept' => 'application/xml, text/xml', + ], + ]); + + if (200 !== $response->getStatusCode()) { + throw new \RuntimeException( + sprintf('unexpected status code %d', $response->getStatusCode()), + 1758000001 + ); + } + + return (string) $response->getBody(); + } + + /** + * Parses the feed and rejects documents that are unusable. Without this an empty or truncated + * response would be imported as an empty feed, and the cleanup would flag every record as deleted. + * + * @param string $xmlContent + * @return \SimpleXMLElement + * @throws \RuntimeException + */ + protected function parseReportsXml($xmlContent) + { + if ('' === trim($xmlContent)) { + throw new \RuntimeException('response is empty', 1758000002); + } + + $useInternalErrors = libxml_use_internal_errors(true); + libxml_clear_errors(); + $xmlData = simplexml_load_string($xmlContent); + $errors = libxml_get_errors(); + libxml_clear_errors(); + libxml_use_internal_errors($useInternalErrors); + + if (false === $xmlData) { + $message = count($errors) > 0 ? trim($errors[0]->message) : 'unknown error'; + throw new \RuntimeException(sprintf('response is not valid XML (%s)', $message), 1758000003); + } + + if (0 === $xmlData->schneemeldung->count()) { + throw new \RuntimeException('response contains no elements', 1758000004); + } + + return $xmlData; + } + + /** + * Loads existing records once, keyed by vendor_uid. Replaces a repository lookup per feed record. + */ + protected function buildIndexes() + { + $this->snowreportIndex = []; + foreach ($this->snowreportRepository->findAll() as $snowreport) { + $this->snowreportIndex[(int) $snowreport->getVendorUid()] = $snowreport; + } + + $this->webcamIndex = []; + foreach ($this->webcamRepository->findAll() as $webcam) { + $this->webcamIndex[(int) $webcam->getVendorUid()] = $webcam; + } + } + + /** + * @param int $vendorUid + * @return Snowreport + */ + protected function getSnowreport($vendorUid) + { + if (isset($this->snowreportIndex[$vendorUid])) { + return $this->snowreportIndex[$vendorUid]; + } + + $snowreport = new Snowreport(); + $snowreport->setVendorUid($vendorUid); + $this->snowreportRepository->add($snowreport); + $this->snowreportIndex[$vendorUid] = $snowreport; + + return $snowreport; + } + + /** + * The service is a singleton, so per-import state must not survive into the next run. + */ + protected function resetImportState() + { + $this->validSnowreportUids = []; + $this->validWebcamUids = []; + $this->conversionWarnings = []; + } + + /** + * Converts a raw XML string to the declared type of the field. + * + * @param string $raw + * @param string $type + * @param string $key XML element name, for logging + * @return int|string|bool + */ + protected function convertValue($raw, $type, $key) + { + $raw = trim($raw); + + // The feed delivers empty elements for data that is not available yet (snow levels out of + // season, for example). That is expected, not an error. + if ($raw === '') { + return $this->getEmptyValue($type); + } + + switch ($type) { + case self::TYPE_STRING: + return $raw; + + case self::TYPE_INT: + if (!preg_match('/^-?\d+$/', $raw)) { + $this->addConversionWarning(sprintf('<%s> expected an integer, got "%s".', $key, $raw)); + + return 0; + } + + return (int) $raw; + + case self::TYPE_BOOL: + $normalized = strtolower($raw); + if (!array_key_exists($normalized, static::$booleanMap)) { + $this->addConversionWarning(sprintf('<%s> expected a boolean, got "%s".', $key, $raw)); + + return false; + } + + return static::$booleanMap[$normalized]; + + case self::TYPE_DATE: + if (!preg_match('/^\d{4}-\d{2}-\d{2}([ T]\d{2}:\d{2}(:\d{2})?)?$/', $raw)) { + $this->addConversionWarning(sprintf('<%s> expected a date, got "%s".', $key, $raw)); + + return 0; + } + + $date = new \DateTime($raw); + + return $date->getTimestamp(); + } + + throw new \InvalidArgumentException(sprintf('Unknown field type "%s" declared for <%s>.', $type, $key), 1758000000); + } + + /** + * @param string $type + * @return int|string|bool + */ + protected function getEmptyValue($type) + { + if ($type === self::TYPE_STRING) { + return ''; + } + if ($type === self::TYPE_BOOL) { + return false; + } + + return 0; + } + + /** + * @param string $message + */ + protected function addConversionWarning($message) + { + // Keyed to keep one entry per distinct problem instead of one per record. + $this->conversionWarnings[$message] = true; + } + + /** + * Writes a single summary entry to the log if the feed contained unexpected values. + */ + protected function logConversionWarnings() + { + if (0 === count($this->conversionWarnings)) { return; } - $GLOBALS['BE_USER']->writelog(4, 0, 0, 0, 'Snowreport XML download failed. Keeping existing file.', []); - @rename($backupPath, $path); + $messages = array_keys($this->conversionWarnings); + $this->writelog(sprintf( + 'Snowreport XML import: %d unexpected value(s) in feed: %s', + count($messages), + implode(' | ', array_slice($messages, 0, 10)) + )); + + $this->conversionWarnings = []; + } + + /** + * The scheduler task and the backend module provide a backend user, the CLI command does not. + * + * @param string $message + */ + protected function writelog($message) + { + if (isset($GLOBALS['BE_USER']) && is_object($GLOBALS['BE_USER'])) { + $GLOBALS['BE_USER']->writelog(4, 0, 0, 0, $message, []); + } } public function getStoragePid() diff --git a/public/typo3conf/ext/ep_products/ext_tables.sql b/public/typo3conf/ext/ep_products/ext_tables.sql index 0b92f9db..e4579c5b 100644 --- a/public/typo3conf/ext/ep_products/ext_tables.sql +++ b/public/typo3conf/ext/ep_products/ext_tables.sql @@ -1098,6 +1098,7 @@ CREATE TABLE tx_epproducts_domain_model_snowreport PRIMARY KEY (uid), KEY parent (pid), + KEY vendor_uid (vendor_uid), KEY t3ver_oid (t3ver_oid, t3ver_wsid), KEY language (l10n_parent, sys_language_uid) @@ -1142,6 +1143,7 @@ CREATE TABLE tx_epproducts_domain_model_webcam PRIMARY KEY (uid), KEY parent (pid), + KEY vendor_uid (vendor_uid), KEY t3ver_oid (t3ver_oid, t3ver_wsid), KEY language (l10n_parent, sys_language_uid)