From 91af156ef954680e55a4c7af508aab0e18180ea9 Mon Sep 17 00:00:00 2001 From: Matt Peel <893117+madmatt@users.noreply.github.com> Date: Thu, 21 Oct 2021 08:39:19 +1300 Subject: [PATCH] FEAT: Allow circular dependency resolution (#26) * FEAT: Allow one level of circular dependency resolution (where one fixture refers to another that hasn't been defined yet) * - Don't loop over all objects and delete individually, as it needlessly creates versions for each deletion, and we TRUNCATE TABLE anyway - Allow recursion over the list of fixtures to allow multiple attempts to create fixtures (still won't help with actual circular dependencies, but will help when A depends on B depends on C, C will get created in the first loop, then B gets created, then A gets created. Script will fail when two subsequent runs over failed fixtures doesn't change the number of fixtures that remain (e.g. we've deadlocked). * Update PopulateFactory to correctly create / modify existing files * Fixing mistake from rebase * Try/catch/log File creation. Linting Co-authored-by: Chris Penny --- code/Populate.php | 15 +- code/PopulateFactory.php | 380 +++++++++++++++++++++++++-------------- 2 files changed, 257 insertions(+), 138 deletions(-) diff --git a/code/Populate.php b/code/Populate.php index 191a007..d910540 100644 --- a/code/Populate.php +++ b/code/Populate.php @@ -3,6 +3,7 @@ namespace DNADesign\Populate; use Exception; +use SilverStripe\Assets\File; use SilverStripe\Control\Director; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Configurable; @@ -10,6 +11,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\YamlFixture; use SilverStripe\ORM\Connect\DatabaseException; +use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\Versioned\Versioned; @@ -76,7 +78,8 @@ public static function requireRecords($force = false): bool throw new Exception('requireRecords can only be run in development or test environments'); } - $factory = Injector::inst()->create('DNADesign\Populate\PopulateFactory'); + /** @var PopulateFactory $factory */ + $factory = Injector::inst()->create(PopulateFactory::class); foreach (self::config()->get('truncate_objects') as $className) { self::truncateObject($className); @@ -87,12 +90,15 @@ public static function requireRecords($force = false): bool } foreach (self::config()->get('include_yaml_fixtures') as $fixtureFile) { + DB::alteration_message(sprintf('Processing %s', $fixtureFile), 'created'); $fixture = new YamlFixture($fixtureFile); $fixture->writeInto($factory); $fixture = null; } + $factory->processFailedFixtures(); + $populate = Injector::inst()->create(Populate::class); $populate->extend('onAfterPopulateRecords'); @@ -106,6 +112,13 @@ public static function requireRecords($force = false): bool */ private static function truncateObject(string $className): void { + if (in_array($className, ClassInfo::subclassesFor(File::class))) { + foreach (DataList::create($className) as $obj) { + /** @var File $obj */ + $obj->deleteFile(); + } + } + $tables = []; // All ancestors or children with tables diff --git a/code/PopulateFactory.php b/code/PopulateFactory.php index 510ba90..a00b0ff 100644 --- a/code/PopulateFactory.php +++ b/code/PopulateFactory.php @@ -3,184 +3,290 @@ namespace DNADesign\Populate; use Exception; -use phpDocumentor\Reflection\DocBlock\Tags\Version; +use InvalidArgumentException; use SilverStripe\Assets\File; use SilverStripe\Assets\Folder; -use SilverStripe\Assets\Upload; -use SilverStripe\Assets\Upload_Validator; -use SilverStripe\Core\Injector\Injector; +use SilverStripe\Assets\Image; +use SilverStripe\Assets\Storage\AssetStore; use SilverStripe\Dev\FixtureBlueprint; use SilverStripe\Dev\FixtureFactory; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DB; use SilverStripe\Versioned\Versioned; +use function basename; +use function dirname; +use function file_get_contents; +use function hash_equals; +use function sha1; +use function sizeof; +use function str_replace; /** * @package populate */ -class PopulateFactory extends FixtureFactory { - - /** - * Creates the object in the database as the original object will be wiped. - * - * @param string $class - * @param string $identifier - * @param array $data - */ - public function createObject($class, $identifier, $data = null) { - DB::alteration_message("Creating $identifier ($class)", "created"); - - if($data) { - foreach($data as $k => $v) { - if(!(is_array($v)) && preg_match('/^`(.)*`;$/', $v)) { - $str = substr($v, 1, -2); - $pv = null; - - eval("\$pv = $str;"); - - $data[$k] = $pv; - } - } - } - - // for files copy the source dir if the image has a 'PopulateFileFrom' - // Follows silverstripe/asset-admin logic, see AssetAdmin::apiCreateFile() - if(isset($data['PopulateFileFrom'])) { - if(!isset($data['Filename'])) { - throw new \Exception('When passing "PopulateFileFrom", you must also pass "Filename" with the path that you want to file to be stored at (e.g. assets/test.jpg)'); +class PopulateFactory extends FixtureFactory +{ + /** + * List of fixtures that failed to be created due to YAML fixture lookup failures (e.g. because of a dependency that + * isn't met at the time of creation). We re-try creation of these after all other fixtures have been created. + * + * @var array + */ + private $failedFixtures = []; + + /** + * Creates the object in the database as the original object will be wiped. + * + * @param string $class + * @param string $identifier + * @param array $data + */ + public function createObject($class, $identifier, $data = null) + { + DB::alteration_message("Creating $identifier ($class)", "created"); + + if ($data) { + foreach ($data as $k => $v) { + if (!(is_array($v)) && preg_match('/^`(.)*`;$/', $v)) { + $str = substr($v, 1, -2); + $pv = null; + + eval("\$pv = $str;"); + + $data[$k] = $pv; + } } + } - $fixtureFilePath = BASE_PATH . '/'. $data['PopulateFileFrom']; - $upload = Upload::create(); - $upload->setReplaceFile(true); - - $folder = Folder::find_or_make( - str_replace('assets/', '', dirname($data['Filename'])) - ); - - $info = new \finfo(FILEINFO_MIME_TYPE); - - $tmpFile = [ - 'name' => isset($data['Name']) ? $data['Name'] : basename($data['Filename']), - 'type' => $info->file($fixtureFilePath), - 'tmp_name' => $fixtureFilePath, - 'error' => 0, - 'size' => filesize($fixtureFilePath) - ]; - - // Disable is_uploaded_file() check in Upload_Validator - $oldUseIsUploadedFile = Upload_Validator::config()->get('use_is_uploaded_file'); - Upload_Validator::config()->set('use_is_uploaded_file', false); - if(!$upload->validate($tmpFile)) { - $errors = $upload->getErrors(); - $message = array_shift($errors); - throw new Exception(sprintf('Error while populating from file %s: %s', $data['Filename'], $message)); - } + // for files copy the source dir if the image has a 'PopulateFileFrom' + // Follows silverstripe/asset-admin logic, see AssetAdmin::apiCreateFile() + if (isset($data['PopulateFileFrom'])) { + $file = $this->populateFile($data); - $fileClass = File::get_class_for_file_extension(File::get_file_extension($tmpFile['name'])); - /** @var File $file */ - $file = Injector::inst()->create($fileClass); + if ($file) { + // Skip the rest of this method (populateFile sets all other values on the object), just return the created file + if (!isset($this->fixtures[$class])) { + $this->fixtures[$class] = []; + } - $uploadResult = $upload->loadIntoFile($tmpFile, $file, $folder->getFilename()); - if (!$uploadResult) { - throw new Exception(sprintf('Failed to load file %s', $data['Filename'])); + $this->fixtures[$class][$identifier] = $file->ID; + return $file; } - Upload_Validator::config()->set('use_is_uploaded_file', $oldUseIsUploadedFile); + } - $file->ParentID = $folder->ID; - $f = $file->toMap(); + // if any merge labels are defined then we should create the object + // from that + $lookup = null; + + if (isset($data['PopulateMergeWhen'])) { + $lookup = DataList::create($class)->where( + $data['PopulateMergeWhen'] + ); + + unset($data['PopulateMergeWhen']); - if ($file->exists()) { - $data['FileHash'] = $f['File']->Hash; - $data['FileFilename'] = $f['File']->Filename; - $data['ParentID'] = $f['File']->ParentID; + } else if (isset($data['PopulateMergeMatch'])) { + $filter = []; + + foreach ($data['PopulateMergeMatch'] as $field) { + $filter[$field] = $data[$field]; } - } + if (!$filter) { + throw new \Exception('Not a valid PopulateMergeMatch filter'); + } - // if any merge labels are defined then we should create the object - // from that - $lookup = null; - $mode = null; + $lookup = DataList::create($class)->filter($filter); - if(isset($data['PopulateMergeWhen'])) { - $mode = 'PopulateMergeWhen'; + unset($data['PopulateMergeMatch']); + } else if (isset($data['PopulateMergeAny'])) { + $lookup = DataList::create($class); - $lookup = DataList::create($class)->where( - $data['PopulateMergeWhen'] - ); + unset($data['PopulateMergeAny']); + } - unset($data['PopulateMergeWhen']); + if ($lookup && $lookup->count() > 0) { + $existing = $lookup->first(); - } else if(isset($data['PopulateMergeMatch'])) { - $mode = 'PopulateMergeMatch'; - $filter = array(); + foreach ($lookup as $old) { + if ($old->ID == $existing->ID) { + continue; + } - foreach($data['PopulateMergeMatch'] as $field) { - $filter[$field] = $data[$field]; - } + if ($old->hasExtension(Versioned::class)) { + foreach ($old->getVersionedStages() as $stage) { + $old->deleteFromStage($stage); + } + } - if(!$filter) { - throw new \Exception('Not a valid PopulateMergeMatch filter'); - } + $old->delete(); + } - $lookup = DataList::create($class)->filter($filter); + $blueprint = new FixtureBlueprint($class); + $obj = $blueprint->createObject($identifier, $data, $this->fixtures); + $latest = $obj->toMap(); - unset($data['PopulateMergeMatch']); - } else if(isset($data['PopulateMergeAny'])) { - $mode = 'PopulateMergeAny'; - $lookup = DataList::create($class); + unset($latest['ID']); - unset($data['PopulateMergeAny']); - } + $existing->update($latest); + $existing->write(); - if($lookup && $lookup->count() > 0) { - $existing = $lookup->first(); + $obj->delete(); - foreach($lookup as $old) { - if($old->ID == $existing->ID) { - continue; - } + $this->fixtures[$class][$identifier] = $existing->ID; - if($old->hasExtension(Versioned::class)) { - foreach($old->getVersionedStages() as $stage) { - $old->deleteFromStage($stage); - } - } + $obj = $existing; + $obj->flushCache(); + } else { + try { + $obj = parent::createObject($class, $identifier, $data); + } catch (InvalidArgumentException $e) { + $this->failedFixtures[] = [ + 'class' => $class, + 'id' => $identifier, + 'data' => $data, + ]; - $old->delete(); - } + DB::alteration_message(sprintf('Exception: %s', $e->getMessage()), 'error'); - $blueprint = new FixtureBlueprint($class); - $obj = $blueprint->createObject($identifier, $data, $this->fixtures); - $latest = $obj->toMap(); + DB::alteration_message( + sprintf('Failed to create %s (%s), queueing for later', $identifier, $class), + 'error' + ); - unset($latest['ID']); + return null; + } + } - $existing->update($latest); - $existing->write(); + if ($obj->hasExtension(Versioned::class)) { + if (Populate::config()->get('enable_publish_recursive')) { + $obj->publishRecursive(); + } else { + $obj->publishSingle(); + } - $obj->delete(); + $obj->flushCache(); + } - $this->fixtures[$class][$identifier] = $existing->ID; + return $obj; + } - $obj = $existing; - $obj->flushCache(); - } - else { - $obj = parent::createObject($class, $identifier, $data); - } + /** + * @param bool $recurse Marker for whether we are recursing - should be false when calling from outside this method + * @throws Exception + */ + public function processFailedFixtures($recurse = false) + { + if (!$this->failedFixtures) { + DB::alteration_message('No failed fixtures to process', 'created'); - if ($obj->hasExtension(Versioned::class)) { - if (Populate::config()->get('enable_publish_recursive')) { - $obj->publishRecursive(); - } else { - $obj->publishSingle(); - } + return; + } - $obj->flushCache(); - } + DB::alteration_message(''); + DB::alteration_message(''); + DB::alteration_message(sprintf('Processing %s failed fixtures', count($this->failedFixtures)), 'created'); - return $obj; - } + $failed = $this->failedFixtures; + + // Reset $this->failedFixtures so that continual failures can be re-attempted + $this->failedFixtures = []; + + foreach ($failed as $fixture) { + // createObject returns null if the object failed to create + // This also re-populates $this->failedFixtures so we can re-compare + $obj = $this->createObject($fixture['class'], $fixture['id'], $fixture['data']); + + if (is_null($obj)) { + DB::alteration_message( + sprintf('Further attempt to create %s (%s) still failed', $fixture['id'], $fixture['class']), + 'error' + ); + } + } + + if (sizeof($this->failedFixtures) > 0 && sizeof($failed) > sizeof($this->failedFixtures)) { + // We made some progress because there are less failed fixtures now than there were before, so run again + $this->processFailedFixtures(true); + } + + // Our final run gets here - either we made no progress on object creation, or there were some fixtures with + // broken or circular relations that can't be resolved - list these at the end. + if (!$recurse && sizeof($this->failedFixtures) > 0) { + $message = sprintf("Some fixtures (%d) couldn't be created:", sizeof($this->failedFixtures)); + DB::alteration_message(""); + DB::alteration_message(""); + DB::alteration_message($message, "error"); + + foreach ($this->failedFixtures as $fixture) { + DB::alteration_message(sprintf('%s (%s)', $fixture['id'], $fixture['class'])); + } + } + } + + /** + * @param array $data + * @return File|bool The created (or updated) File object + * @throws Exception If anything is missing and the file can't be processed + */ + private function populateFile($data) + { + if (!isset($data['Filename']) || !isset($data['PopulateFileFrom'])) { + throw new Exception('When passing "PopulateFileFrom", you must also pass "Filename" with the path that you want to file to be stored at (e.g. assets/test.jpg)'); + } + + $fixtureFilePath = BASE_PATH . '/' . $data['PopulateFileFrom']; + $filenameWithoutAssets = str_replace('assets/', '', $data['Filename']); + + // Find the existing object (if one exists) + /** @var File $existingObj */ + $existingObj = File::find($filenameWithoutAssets); + + if ($existingObj && $existingObj->exists()) { + $file = $existingObj; + + // If the file hashes match, and the file already exists, we don't need to update anything. + $hash = $existingObj->File->getHash(); + + if (hash_equals($hash, sha1(file_get_contents($fixtureFilePath)))) { + return true; + } + } else { + $file = Image::create(); + } + + $folder = Folder::find_or_make(dirname($filenameWithoutAssets)); + $filename = basename($filenameWithoutAssets); + + // We could just use $data['Filename'], but we need to allow for filsystem abstraction + $filePath = File::join_paths($folder->getFilename(), $filename); + + $fileCfg = [ + // if there's a filename conflict we've got new content so overwrite it. + 'conflict' => AssetStore::CONFLICT_OVERWRITE, + 'visibility' => AssetStore::VISIBILITY_PUBLIC, + ]; + + // Set any other attributes that the file may need (e.g. Title) + foreach ($data as $k => $v) { + if (in_array($k, ['PopulateFileFrom', 'Filename'])) { + continue; + } + + $file->$k = $v; + } + + try { + $file->setFromString(file_get_contents($fixtureFilePath), $filePath, null, null, $fileCfg); + // Setting ParentID needs to come after setFromString() as (at least sometimes) setFromString() resets the + // file Parent back to the "Uploads" folder + $file->ParentID = $folder->ID; + $file->write(); + $file->publishRecursive(); + } catch (Exception $e) { + DB::alteration_message($e->getMessage(), "error"); + } + + return $file; + } }