From 99b20ba5be06fa87d3fcd47e877a128ea9bfcf0c Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Tue, 23 Dec 2014 01:53:20 +0800 Subject: [PATCH] More biz to help simfiles make it. Better error reporting service. --- Controllers/SimfileController.php | 2 +- DataAccess/DataMapper/DataMapper.php | 245 +++++++++++---------- .../Helpers/AbstractPopulationHelper.php | 3 +- Domain/VOs/StepMania/DanceMode.php | 2 + Services/IStatusReporter.php | 5 +- Services/SimfileParser.php | 38 +++- Services/StatusReporter.php | 23 +- Services/Uploads/UploadManager.php | 2 +- Services/ZipParser.php | 32 ++- 9 files changed, 211 insertions(+), 141 deletions(-) diff --git a/Controllers/SimfileController.php b/Controllers/SimfileController.php index 582cfc0..5e83f9f 100644 --- a/Controllers/SimfileController.php +++ b/Controllers/SimfileController.php @@ -140,7 +140,7 @@ class SimfileController implements IDivineController $zipParser->parse($file); if(!$zipParser->simfiles()) $this->_statusReporter->error('That zip doesn\'t seem to have any simfiles in it.'); - + //save the actual zip in the db $this->findAndAddSmoMirror($file); $this->_fileRepository->save($file); diff --git a/DataAccess/DataMapper/DataMapper.php b/DataAccess/DataMapper/DataMapper.php index b5217ad..3900522 100644 --- a/DataAccess/DataMapper/DataMapper.php +++ b/DataAccess/DataMapper/DataMapper.php @@ -2,6 +2,7 @@ namespace DataAccess\DataMapper; +use Exception; use Domain\Entities\IDivineEntity; use DataAccess\IDatabaseFactory; use DataAccess\DataMapper\IDataMapper; @@ -64,146 +65,160 @@ class DataMapper implements IDataMapper public function save(IDivineEntity $entity) { - $queries = AbstractPopulationHelper::generateUpdateSaveQuery($this->_maps, $entity, $entity->getId(), $this->_db); - $mergeMap = array(); - $flattened = array(); + try { + $this->_db->beginTransaction(); + $queries = AbstractPopulationHelper::generateUpdateSaveQuery($this->_maps, $entity, $entity->getId(), $this->_db); + $mergeMap = array(); + $flattened = array(); - foreach($queries as $index => $query) - { - $this_table = $query['table']; - $this_columns = $query['columns']; - - - for($i = $index+1; $i $query) { - if( - $queries[$i]['table'] == $this_table && - !array_key_exists($i, $mergeMap) && - !isset($query['id'])) //only merge create queries, updates are fine to run multiple times + $this_table = $query['table']; + $this_columns = $query['columns']; + + + for($i = $index+1; $i $this_columns, + 'table' => $this_table, + 'prepared' => $prepared, + 'id' => $id + ); + } } - - if(!array_key_exists($index, $mergeMap)) { - $prepared = isset($query['prepared']) ? $query['prepared'] : null; - $id = isset($query['id']) ? $query['id'] : null; - - $flattened[$index] = array( - 'columns' => $this_columns, - 'table' => $this_table, - 'prepared' => $prepared, - 'id' => $id - ); - } - } - - $queries = array(); - - foreach($flattened as $index => $info) - { - if(isset($info['id'])) + + $queries = array(); + + foreach($flattened as $index => $info) { - $query = $info['prepared']; - $query = substr($query, 0, -2); - $query .= sprintf(' WHERE id=%u', $info['id']); - } else { - $query = sprintf('INSERT INTO %s (%s) VALUES (%s)', - $info['table'], - implode(', ', array_keys($info['columns'])), - implode(', ', $info['columns'])); + if(isset($info['id'])) + { + $query = $info['prepared']; + $query = substr($query, 0, -2); + $query .= sprintf(' WHERE id=%u', $info['id']); + } else { + $query = sprintf('INSERT INTO %s (%s) VALUES (%s)', + $info['table'], + implode(', ', array_keys($info['columns'])), + implode(', ', $info['columns'])); + } + + $queries[$index] = $query; } - $queries[$index] = $query; - } + // if($queries['TYPE'] == AbstractPopulationHelper::QUERY_TYPE_CREATE) + // { + $idMap = []; + foreach($queries as $index => $query) + { + $runQuery = true; + //originally was preg_quote('%').'(.*?)'.preg_quote('%') but that failed with things like: + //...VALUES ('Voyager Full 50%', %INDEX_REF_0% + //it picked up ', + //so now only find ones with INDEX_REF and double check that MAIN QUERY isn't there. + if (preg_match_all('/'.preg_quote('%INDEX_REF_').'(.*?)'.preg_quote('%').'/s', $query, $matches)) { + foreach($matches[1] as $index_ref) + { + //if($index_ref != 'MAIN_QUERY_ID') + //if(strpos($query, '%MAIN_QUERY_ID%') === false) + //{ + $index_id = str_replace('INDEX_REF_', '', $index_ref); + $query = str_replace('%INDEX_REF_' . $index_id . '%', $idMap['INDEX_REF_' . $index_id], $query); + //} else { + // $runQuery = false; + //} + } + } - // if($queries['TYPE'] == AbstractPopulationHelper::QUERY_TYPE_CREATE) - // { - $idMap = []; - foreach($queries as $index => $query) - { - $runQuery = true; - if (preg_match_all('/'.preg_quote('%').'(.*?)'.preg_quote('%').'/s', $query, $matches)) { - foreach($matches[1] as $index_ref) + //if we don't need the main query we can run this + if(strpos($query, '%MAIN_QUERY_ID%') === false) { - if($index_ref != 'MAIN_QUERY_ID') - { - $index_id = str_replace('INDEX_REF_', '', $index_ref); - $query = str_replace('%INDEX_REF_' . $index_id . '%', $idMap['INDEX_REF_' . $index_id], $query); - } else { - $runQuery = false; + $statement = $this->_db->prepare($query); + $statement->execute(); + //$refIndex = $index+1; This was being used as the index for idMap below. I have nfi why I was adding 1. + $idMap['INDEX_REF_' . $index] = $this->_db->lastInsertId(); + + foreach($mergeMap as $oldIndex => $mergedIndex) { + if($mergedIndex == $index) { + $idMap['INDEX_REF_' . $oldIndex] = $idMap['INDEX_REF_' . $index]; + } } + + unset($queries[$index]); + } else { + //update query so that other references are resolved. + $queries[$index] = $query; } } - if($runQuery) + //at this point we have queries left that depend on the main query id + foreach($queries as $query) { + $query = str_replace('%MAIN_QUERY_ID%', end($idMap), $query); $statement = $this->_db->prepare($query); $statement->execute(); - //$refIndex = $index+1; This was being used as the index for idMap below. I have nfi why I was adding 1. - $idMap['INDEX_REF_' . $index] = $this->_db->lastInsertId(); - - foreach($mergeMap as $oldIndex => $mergedIndex) { - if($mergedIndex == $index) { - $idMap['INDEX_REF_' . $oldIndex] = $idMap['INDEX_REF_' . $index]; - } - } - - unset($queries[$index]); - } else { - //update query so that other references are resolved. - $queries[$index] = $query; } - } + //} + + if(!$entity->getId()) $entity->setId(end($idMap)); - //at this point we have queries left that depend on the main query id - foreach($queries as $query) - { - $query = str_replace('%MAIN_QUERY_ID%', end($idMap), $query); - $statement = $this->_db->prepare($query); - $statement->execute(); - } - //} - - if(!$entity->getId()) $entity->setId(end($idMap)); - - return $entity; + $this->_db->commit(); + + return $entity; + } catch (Exception $e) { + $this->_db->rollBack(); + throw $e; + } } //TODO: Implement diff --git a/DataAccess/DataMapper/Helpers/AbstractPopulationHelper.php b/DataAccess/DataMapper/Helpers/AbstractPopulationHelper.php index a2f1ea6..00a442a 100644 --- a/DataAccess/DataMapper/Helpers/AbstractPopulationHelper.php +++ b/DataAccess/DataMapper/Helpers/AbstractPopulationHelper.php @@ -373,7 +373,8 @@ class AbstractPopulationHelper foreach($columns as $columnName => $columnValue) { - $query .= sprintf('%s="%s" AND ', $columnName, str_replace('"', '\"', $columnValue)); + $columnValue = $db->quote($columnValue); + $query .= sprintf('%s=%s AND ', $columnName, str_replace('"', '\"', $columnValue)); } $query = substr($query, 0, -4); diff --git a/Domain/VOs/StepMania/DanceMode.php b/Domain/VOs/StepMania/DanceMode.php index 34e0309..582b9ca 100644 --- a/Domain/VOs/StepMania/DanceMode.php +++ b/Domain/VOs/StepMania/DanceMode.php @@ -56,6 +56,7 @@ class DanceMode implements IDanceMode 'techno-double8' => 'Double 8 Panel', 'pnm-five' => 'Five Key', 'pnm-nine' => 'Nine Key', + 'guitar-five' => '5 Fret', 'lights-cabinet' => 'Cabinet Lights' ); @@ -71,6 +72,7 @@ class DanceMode implements IDanceMode 'pnm' => 'Pop\'n Music', 'ds3ddx' => 'Dance Station 3DDX', 'kb7' => 'Keybeat', + 'guitar' => 'Guitar Hero', 'lights' => false ); diff --git a/Services/IStatusReporter.php b/Services/IStatusReporter.php index 3214a41..b88c70b 100644 --- a/Services/IStatusReporter.php +++ b/Services/IStatusReporter.php @@ -6,8 +6,9 @@ use Exception; interface IStatusReporter { - public function success($message); - public function error($message); + public function success($message = null); + public function error($message = null); + public function addMessage($message); public static function exception(Exception $exception); public function json(); } diff --git a/Services/SimfileParser.php b/Services/SimfileParser.php index c56dc6d..374bc17 100644 --- a/Services/SimfileParser.php +++ b/Services/SimfileParser.php @@ -2,13 +2,22 @@ namespace Services; -use Services\ISimfileParser; use Exception; +use Services\ISimfileParser; +use Services\IStatusReporter; + +class InvalidSmFileException extends Exception{} class SimfileParser implements ISimfileParser { private $_smFileLines; + private $_statusReporter; + + public function __construct(IStatusReporter $statusReporter) + { + $this->_statusReporter = $statusReporter; + } public function parse($simfileData) { @@ -23,7 +32,7 @@ class SimfileParser implements ISimfileParser public function title() { $title = $this->extractKey('TITLE'); - if(!$title) throw new Exception ('Invalid SM file. TITLE missing'); + if(!$title) throw new InvalidSmFileException('Invalid SM file. TITLE missing'); //XXX: UTF8 encode to deal with unusual character that crop up in weeaboo shit. return utf8_encode($title); @@ -65,7 +74,7 @@ class SimfileParser implements ISimfileParser if($displayBpm) { $bpmRange = explode(":",$displayBpm); - $bpmRange[1] = @$bpmRange[1] ?: $bpmRange[0]; + $bpmRange[1] = isset($bpmRange[1]) && is_numeric($bpmRange[1]) ?: $bpmRange[0]; } //XXX: Originally I had an else statement for this. But turns out some SM's have * as the display bpm @@ -105,7 +114,7 @@ class SimfileParser implements ISimfileParser public function subtitle() { $subtitle = $this->extractKey('SUBTITLE'); - if(!$subtitle) throw new Exception ('Invalid SM file. SUBTITLE missing'); + if(!$subtitle) return null; return $subtitle; } @@ -122,14 +131,14 @@ class SimfileParser implements ISimfileParser { $noteData = trim(substr($line, $pos + 9)); $steps = $this->stepchartFromNoteData($noteData); - + //XXX: Sometimes we get a cabinet lights chart, those return false for getGame. //We don't want to store cabinet lights, so just ignore it. if($steps->getMode()->getGame()) $allSteps[] = $steps; } } - if(empty($allSteps)) throw new Exception('Invalid Sm file. NOTES missing'); + if(empty($allSteps)) throw new InvalidSmFileException('Invalid Sm file. NOTES missing'); return $allSteps; } @@ -139,13 +148,15 @@ class SimfileParser implements ISimfileParser return new \Domain\VOs\StepMania\StepChart( new \Domain\VOs\StepMania\DanceMode($stepData[0]), new \Domain\VOs\StepMania\Difficulty($stepData[2]), - empty($stepData[1]) ? null : new \Domain\VOs\StepMania\StepArtist($stepData[1]), - $stepData[3] + empty($stepData[1]) ? null : new \Domain\VOs\StepMania\StepArtist(utf8_encode($stepData[1])), + //XXX: Fuck you whoever made me do this. http://dev.mysql.com/doc/refman/5.5/en/integer-types.html + $stepData[3] <= 18446744073709551615 ? $stepData[3] : 9999999999999999999 ); } private function extractKey($key) { + //Throw regular exception here, this has nothing to do with the SM file. if(empty($this->_smFileLines)) throw new Exception('SM file data not set.'); foreach ($this->_smFileLines as $line) @@ -164,13 +175,18 @@ class SimfileParser implements ISimfileParser foreach($bpms as $bpm) { - $bpmMeasure = explode('=', $bpm); - $bpmValue = floatval($bpmMeasure[1]); + $bpmMeasure = explode('=', $bpm); + $bpmValue = floatval($bpmMeasure[1]); - if(empty($bpmRange['low'])) $bpmRange['low'] = $bpmRange['high'] = $bpmValue; + if(empty($bpmRange['low'])) $bpmRange['low'] = $bpmRange['high'] = $bpmValue; + //XXX: Anything bigger or smaller than this cannot be stored by MySQLs bigint type http://dev.mysql.com/doc/refman/5.5/en/integer-types.html + //fuck you if your simfile has bpms this high/low what's wrong with you. + if($bpmValue <= 9223372036854775807 && $bpmValue >= -9223372036854775808) + { $bpmRange['high'] = ($bpmValue > $bpmRange['high']) ? $bpmValue : $bpmRange['high']; $bpmRange['low'] = ($bpmValue < $bpmRange['low']) ? $bpmValue : $bpmRange['low']; + } } return array($bpmRange['low'], $bpmRange['high']); diff --git a/Services/StatusReporter.php b/Services/StatusReporter.php index 3667c88..32c9481 100644 --- a/Services/StatusReporter.php +++ b/Services/StatusReporter.php @@ -8,7 +8,7 @@ use Services\IStatusReporter; class StatusReporter implements IStatusReporter { - private $_message; + private $_messages = array(); private $_type; private $_response; @@ -20,9 +20,9 @@ class StatusReporter implements IStatusReporter $this->_response = $response; } - public function error($message) + public function error($message = null) { - $this->_message = $message; + if($message) $this->addMessage($message); $this->_type = self::ERROR; $this->_response->setHeader('Content-Type', 'application/json') ->setBody($this->json()) @@ -30,9 +30,9 @@ class StatusReporter implements IStatusReporter exit(); } - public function success($message) + public function success($message = null) { - $this->_message = $message; + if($message) $this->addMessage($message); $this->_type = self::SUCCESS; $this->_response->setHeader('Content-Type', 'application/json') ->setBody($this->json()) @@ -40,18 +40,27 @@ class StatusReporter implements IStatusReporter exit(); } + public function addMessage($message) + { + $this->_messages[] = $message;; + } + //no need to exit here, exceptions stop the program. public static function exception(Exception $exception) { //we'll be instatic context here so I have to do it this way. header('Content-Type: application/json'); - echo json_encode(array(self::EXCEPTION => $exception->getMessage())); + echo json_encode(array( + 'status' => self::EXCEPTION, + 'messages' => array($exception->getMessage()))); } public function json() { return json_encode( - array($this->_type => $this->_message) + array( + 'status' => $this->_type, + 'messages' => $this->_messages) ); } } diff --git a/Services/Uploads/UploadManager.php b/Services/Uploads/UploadManager.php index c1356ae..81a6413 100644 --- a/Services/Uploads/UploadManager.php +++ b/Services/Uploads/UploadManager.php @@ -26,7 +26,7 @@ class UploadManager implements IUploadManager{ foreach($_FILES as $file) { $this->_files[] = $this->_fileFactory->createInstance( - $file['name'], + utf8_encode($file['name']), $file['type'], $file['tmp_name'], $file['size'] diff --git a/Services/ZipParser.php b/Services/ZipParser.php index 2fecc25..380a6c4 100644 --- a/Services/ZipParser.php +++ b/Services/ZipParser.php @@ -3,6 +3,8 @@ namespace Services; use Exception; +use Domain\Exception\InvalidDifficultyException; +use Domain\Exception\InvalidDanceModeException; use ZipArchive; use Services\ISimfileParser; use Services\IBannerExtracter; @@ -11,6 +13,8 @@ use Domain\Entities\StepMania\ISimfileStepByStepBuilder; use Domain\Entities\StepMania\IPackStepByStepBuilder; use Services\IZipParser; use Services\IUserSession; +use Services\IStatusReporter; +use Services\InvalidSmFileException; class ZipParser implements IZipParser { @@ -22,19 +26,22 @@ class ZipParser implements IZipParser private $_bannerExtracter; private $_userSession; private $_file; + private $_statusReporter; public function __construct( ISimfileParser $smParser, ISimfileStepByStepBuilder $smBuilder, IPackStepByStepBuilder $packBuilder, IBannerExtracter $bannerExtracter, - IUserSession $userSession + IUserSession $userSession, + IStatusReporter $statusReporter ) { $this->_smParser = $smParser; $this->_smBuilder = $smBuilder; $this->_packBuilder = $packBuilder; $this->_bannerExtracter = $bannerExtracter; $this->_userSession = $userSession; + $this->_statusReporter = $statusReporter; } public function parse(IFile $file) @@ -99,7 +106,26 @@ class ZipParser implements IZipParser //or single, but to do that we simply check the number of found sm files. To overcome this //first populate the smFiles array with the raw sm data, then apply SmDataToSmClass on each //array element. This way the check is accurate and the array gets populated as expected. - $this->_smFiles = array_map(array($this, 'SmDataToSmClass'), $this->_smFiles); + foreach($this->_smFiles as $index => $data) + { + try + { + $this->_smFiles[$index] = $this->SmDataToSmClass($data); + } catch(Exception $e) { + //Exceptions we care about at this stage + if(!$e instanceof InvalidSmFileException && + !$e instanceof InvalidDanceModeException && + !$e instanceof InvalidDifficultyException) + { + throw $e; + } + + //Add to messages. + $this->_statusReporter->addMessage($e->getMessage() . ' in ' . $index); + unset($this->_smFiles[$index]); + } + } + //$this->_smFiles = array_map(array($this, 'SmDataToSmClass'), $this->_smFiles); } private function packNameFromFiles() @@ -125,7 +151,7 @@ class ZipParser implements IZipParser $parser->parse($smData); $banner = $this->_bannerExtracter->extractSongBanner(realpath('../files/StepMania/' . $this->_file->getHash() . '.zip'), $parser->banner()); $file = $this->isPack() ? null : $this->_file; - + return $this->_smBuilder->With_Title($parser->title()) ->With_Artist($parser->artist()) ->With_Uploader($this->_userSession->getCurrentUser()) //obj -- 2.11.0