fix: make snowreport import type-safe and drop wget shell-out

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 <schneemeldung> 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()

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)
 

temp/.gitignore

@@ -0,0 +1,2 @@
+*
+!.gitignore