From 2bd9d00da0187558b7c76cd0bdc9d9664c140572 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 15 Oct 2015 11:08:52 +1300 Subject: [PATCH] API Remove filesystem sync API support new asset abstraction BUG Fix file link tracking for new asset abstraction --- code/controllers/AssetAdmin.php | 240 +++++-------------- code/model/SiteTree.php | 43 ++-- code/model/SiteTreeFileExtension.php | 74 +++--- code/model/SiteTreeLinkTracking.php | 43 ++-- tasks/FilesystemSyncTask.php | 25 -- tests/behat/features/insert-a-link.feature | 10 +- tests/behat/features/insert-an-image.feature | 22 +- tests/behat/features/manage-files.feature | 4 +- tests/model/FileLinkTrackingTest.php | 95 +++++--- tests/model/FileLinkTrackingTest.yml | 16 +- tests/model/SiteTreeHTMLEditorFieldTest.php | 36 ++- tests/model/SiteTreeHtmlEditorFieldTest.yml | 4 + tests/model/SiteTreeLinkTrackingTest.php | 20 +- 13 files changed, 271 insertions(+), 361 deletions(-) delete mode 100644 tasks/FilesystemSyncTask.php diff --git a/code/controllers/AssetAdmin.php b/code/controllers/AssetAdmin.php index 0e32d6f3..573d1213 100644 --- a/code/controllers/AssetAdmin.php +++ b/code/controllers/AssetAdmin.php @@ -1,4 +1,7 @@ 'ADMIN', - 'doSync', - 'filter', + 'getsubtree' ); /** @@ -62,13 +58,11 @@ class AssetAdmin extends LeftAndMain implements PermissionProvider{ } /** - * Set up the controller, in particular, re-sync the File database with the assets folder./ + * Set up the controller */ public function init() { parent::init(); - // Create base folder if it doesnt exist already - if(!file_exists(ASSETS_PATH)) Filesystem::makeFolder(ASSETS_PATH); Requirements::javascript(CMS_DIR . "/javascript/AssetAdmin.js"); Requirements::javascript(CMS_DIR . '/javascript/CMSMain.GridField.js'); @@ -210,20 +204,6 @@ JS $addFolderBtn = ''; } - if($folder->canEdit()) { - $syncButton = new LiteralField( - 'SyncButton', - sprintf( - '%s', - _t('AssetAdmin.FILESYSTEMSYNCTITLE', 'Update the CMS database entries of files on the filesystem. Useful when new files have been uploaded outside of the CMS, e.g. through FTP.'), - $this->Link('doSync'), - _t('AssetAdmin.FILESYSTEMSYNC','Sync files') - ) - ); - } else { - $syncButton = null; - } - // Move existing fields to a "details" tab, unless they've already been tabbed out through extensions. // Required to keep Folder->getCMSFields() simple and reuseable, // without any dependencies into AssetAdmin (e.g. useful for "add folder" views). @@ -249,7 +229,6 @@ JS // the button shouldn't be available. Adding empty values into a ComposteField breaks template rendering. $actionButtonsComposite = CompositeField::create()->addExtraClass('cms-actions-row'); if($addFolderBtn) $actionButtonsComposite->push($addFolderBtn); - if($syncButton) $actionButtonsComposite->push($syncButton); // Add the upload field for new media if($currentPageID = $this->currentPageID()){ @@ -266,9 +245,8 @@ JS $uploadField->removeExtraClass('ss-uploadfield'); $uploadField->setTemplate('AssetUploadField'); - if($folder->exists() && $folder->getFilename()) { - // The Upload class expects a folder relative *within* assets/ - $path = preg_replace('/^' . ASSETS_DIR . '\//', '', $folder->getFilename()); + if($folder->exists()) { + $path = $folder->getFilename(); $uploadField->setFolderName($path); } else { $uploadField->setFolderName('/'); // root of the assets @@ -287,8 +265,7 @@ JS new HiddenField('ID'), $gridField )); - - $treeField = new LiteralField('Tree', ''); + // Tree view $fields->addFieldsToTab('Root.TreeView', array( clone $actionsComposite, @@ -320,8 +297,6 @@ JS ); } - - $fields->setForm($form); $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); // TODO Can't merge $FormAttributes in template at the moment @@ -353,8 +328,12 @@ JS $className = $this->stat('tree_class'); $record = DataObject::get_by_id($className, $data['ID']); - if($record && !$record->canDelete()) return Security::permissionFailure(); - if(!$record || !$record->ID) throw new SS_HTTPResponse_Exception("Bad record ID #" . (int)$data['ID'], 404); + if($record && !$record->canDelete()) { + return Security::permissionFailure(); + } + if(!$record || !$record->ID) { + throw new SS_HTTPResponse_Exception("Bad record ID #" . (int)$data['ID'], 404); + } $parentID = $record->ParentID; $record->delete(); $this->setCurrentPageID(null); @@ -373,8 +352,12 @@ JS $context = singleton('File')->getDefaultSearchContext(); // Namespace fields, for easier detection if a search is present - foreach($context->getFields() as $field) $field->setName(sprintf('q[%s]', $field->getName())); - foreach($context->getFilters() as $filter) $filter->setFullName(sprintf('q[%s]', $filter->getFullName())); + foreach($context->getFields() as $field) { + $field->setName(sprintf('q[%s]', $field->getName())); + } + foreach($context->getFilters() as $filter) { + $filter->setFullName(sprintf('q[%s]', $filter->getFullName())); + } // Customize fields $dateHeader = HeaderField::create('q[Date]', _t('CMSSearch.FILTERDATEHEADING', 'Date'), 4); @@ -389,12 +372,12 @@ JS ); $context->addField($dateGroup); $appCategories = array( - 'image' => _t('AssetAdmin.AppCategoryImage', 'Image'), + 'archive' => _t('AssetAdmin.AppCategoryArchive', 'Archive', 'A collection of files'), 'audio' => _t('AssetAdmin.AppCategoryAudio', 'Audio'), - 'mov' => _t('AssetAdmin.AppCategoryVideo', 'Video'), + 'document' => _t('AssetAdmin.AppCategoryDocument', 'Document'), 'flash' => _t('AssetAdmin.AppCategoryFlash', 'Flash', 'The fileformat'), - 'zip' => _t('AssetAdmin.AppCategoryArchive', 'Archive', 'A collection of files'), - 'doc' => _t('AssetAdmin.AppCategoryDocument', 'Document') + 'image' => _t('AssetAdmin.AppCategoryImage', 'Image'), + 'video' => _t('AssetAdmin.AppCategoryVideo', 'Video'), ); $context->addField( $typeDropdown = new DropdownField( @@ -444,7 +427,6 @@ JS } public function AddForm() { - $folder = singleton('Folder'); $form = CMSForm::create( $this, 'AddForm', @@ -475,7 +457,9 @@ JS $class = $this->stat('tree_class'); // check create permissions - if(!singleton($class)->canCreate()) return Security::permissionFailure($this); + if(!singleton($class)->canCreate()) { + return Security::permissionFailure($this); + } // check addchildren permissions if( @@ -485,44 +469,40 @@ JS && $data['ParentID'] ) { $parentRecord = DataObject::get_by_id($class, $data['ParentID']); - if( - $parentRecord->hasMethod('canAddChildren') - && !$parentRecord->canAddChildren() - ) return Security::permissionFailure($this); + if($parentRecord->hasMethod('canAddChildren') && !$parentRecord->canAddChildren()) { + return Security::permissionFailure($this); + } } else { $parentRecord = null; } - $parent = (isset($data['ParentID']) && is_numeric($data['ParentID'])) ? (int)$data['ParentID'] : 0; - $name = (isset($data['Name'])) ? basename($data['Name']) : _t('AssetAdmin.NEWFOLDER',"NewFolder"); - if(!$parentRecord || !$parentRecord->ID) $parent = 0; + // Check parent + $parentID = $parentRecord && $parentRecord->ID + ? (int)$parentRecord->ID + : 0; + // Build filename + $filename = isset($data['Name']) + ? basename($data['Name']) + : _t('AssetAdmin.NEWFOLDER',"NewFolder"); + if($parentRecord && $parentRecord->ID) { + $filename = $parentRecord->getFilename() . '/' . $filename; + } // Get the folder to be created - if($parentRecord && $parentRecord->ID) $filename = $parentRecord->FullPath . $name; - else $filename = ASSETS_PATH . '/' . $name; - // Actually create - if(!file_exists(ASSETS_PATH)) { - mkdir(ASSETS_PATH); + // Ensure name is unique + foreach($this->getNameGenerator($filename) as $filename) { + if(! File::find($filename) ) { + break; + } } - $record = new Folder(); - $record->ParentID = $parent; + // Create record + $record = Folder::create(); + $record->ParentID = $parentID; $record->Name = $record->Title = basename($filename); - - // Ensure uniqueness - $i = 2; - $baseFilename = substr($record->Filename, 0, -1) . '-'; - while(file_exists($record->FullPath)) { - $record->Filename = $baseFilename . $i . '/'; - $i++; - } - - $record->Name = $record->Title = basename($record->Filename); $record->write(); - mkdir($record->FullPath); - chmod($record->FullPath, Filesystem::config()->file_create_mask); if($parentRecord) { return $this->redirect(Controller::join_links($this->Link('show'), $parentRecord->ID)); @@ -531,6 +511,17 @@ JS } } + /** + * Get an asset renamer for the given filename. + * + * @param string $filename Path name + * @return AssetNameGenerator + */ + protected function getNameGenerator($filename){ + return Injector::inst() + ->createWithArgs('AssetNameGenerator', array($filename)); + } + /** * Custom currentPage() method to handle opening the 'root' folder */ @@ -560,117 +551,6 @@ JS return $this->getSiteTreeFor($this->stat('tree_class'), null, 'ChildFolders', 'numChildFolders'); } - //------------------------------------------------------------------------------------------// - - // Data saving handlers - - /** - * Can be queried with an ajax request to trigger the filesystem sync. It returns a FormResponse status message - * to display in the CMS - */ - public function doSync() { - $message = Filesystem::sync(); - $this->response->addHeader('X-Status', rawurlencode($message)); - - return; - } - - /** - * ################################# - * Garbage collection. - * ################################# - */ - - /** - * Removes all unused thumbnails from the file store - * and returns the status of the process to the user. - */ - public function deleteunusedthumbnails($request) { - // Protect against CSRF on destructive action - if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); - - $count = 0; - $thumbnails = $this->getUnusedThumbnails(); - - if($thumbnails) { - foreach($thumbnails as $thumbnail) { - unlink(ASSETS_PATH . "/" . $thumbnail); - $count++; - } - } - - $message = _t( - 'AssetAdmin.THUMBSDELETED', - '{count} unused thumbnails have been deleted', - array('count' => $count) - ); - $this->response->addHeader('X-Status', rawurlencode($message)); - return; - } - - /** - * Creates array containg all unused thumbnails. - * - * Array is created in three steps: - * 1. Scan assets folder and retrieve all thumbnails - * 2. Scan all HTMLField in system and retrieve thumbnails from them. - * 3. Count difference between two sets (array_diff) - * - * @return array - */ - private function getUnusedThumbnails() { - $allThumbnails = array(); - $usedThumbnails = array(); - $dirIterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator(ASSETS_PATH)); - $classes = ClassInfo::subclassesFor('SiteTree'); - - if($dirIterator) { - foreach($dirIterator as $file) { - if($file->isFile()) { - if(strpos($file->getPathname(), '_resampled') !== false) { - $pathInfo = pathinfo($file->getPathname()); - if(in_array(strtolower($pathInfo['extension']), array('jpeg', 'jpg', 'jpe', 'png', 'gif'))) { - $path = str_replace('\\','/', $file->getPathname()); - $allThumbnails[] = substr($path, strpos($path, '/assets/') + 8); - } - } - } - } - } - - if($classes) { - foreach($classes as $className) { - $SNG_class = singleton($className); - $objects = DataObject::get($className); - - if($objects !== NULL) { - foreach($objects as $object) { - foreach($SNG_class->db() as $fieldName => $fieldType) { - if($fieldType == 'HTMLText') { - $url1 = HTTP::findByTagAndAttribute($object->$fieldName,array('img' => 'src')); - - if($url1 != NULL) { - $usedThumbnails[] = substr($url1[0], strpos($url1[0], '/assets/') + 8); - } - - if($object->latestPublished > 0) { - $object = Versioned::get_latest_version($className, $object->ID); - $url2 = HTTP::findByTagAndAttribute($object->$fieldName, array('img' => 'src')); - - if($url2 != NULL) { - $usedThumbnails[] = substr($url2[0], strpos($url2[0], '/assets/') + 8); - } - } - } - } - } - } - } - } - - return array_diff($allThumbnails, $usedThumbnails); - } - /** * @param bool $unlinked * @return ArrayList diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index 3682d5e2..3b1e1519 100755 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -1764,10 +1764,12 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid } /** - * Rewrite a file URL on this page, after its been renamed. Triggers the onRenameLinkedAsset action on extensions. + * Rewrites any linked images on this page. + * Non-image files should be linked via shortcodes + * Triggers the onRenameLinkedAsset action on extensions. + * TODO: This doesn't work for HTMLText fields on other tables. */ - public function rewriteFileURL($old, $new) { - $fields = $this->inheritedDatabaseFields(); + public function rewriteFileLinks() { // Update the content without actually creating a new version foreach(array("SiteTree_Live", "SiteTree") as $table) { // Published site @@ -1777,23 +1779,26 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid )->record(); $origPublished = $published; - foreach($fields as $fieldName => $fieldType) { - if ($fieldType != 'HTMLText') continue; + foreach($this->db() as $fieldName => $fieldType) { + // Skip if non HTML or if empty + if ($fieldType !== 'HTMLText' || empty($published[$fieldName])) { + continue; + } - // TODO: This doesn't work for HTMLText fields on other tables. - if(isset($published[$fieldName])) { - $published[$fieldName] = str_replace($old, $new, $published[$fieldName], $numReplaced); - if($numReplaced) { - $query = sprintf('UPDATE "%s" SET "%s" = ? WHERE "ID" = ?', $table, $fieldName); - DB::prepared_query($query, array($published[$fieldName], $this->ID)); - - // Tell static caching to update itself - if($table == 'SiteTree_Live') { - $publishedClass = $origPublished['ClassName']; - $origPublishedObj = new $publishedClass($origPublished); - $this->invokeWithExtensions('onRenameLinkedAsset', $origPublishedObj); - } - } + // Regenerate content + $content = Image::regenerate_html_links($published[$fieldName]); + if($content === $published[$fieldName]) { + continue; + } + + $query = sprintf('UPDATE "%s" SET "%s" = ? WHERE "ID" = ?', $table, $fieldName); + DB::prepared_query($query, array($content, $this->ID)); + + // Tell static caching to update itself + if($table == 'SiteTree_Live') { + $publishedClass = $origPublished['ClassName']; + $origPublishedObj = new $publishedClass($origPublished); + $this->invokeWithExtensions('onRenameLinkedAsset', $origPublishedObj); } } } diff --git a/code/model/SiteTreeFileExtension.php b/code/model/SiteTreeFileExtension.php index 12b6c9c7..7b7e29f6 100644 --- a/code/model/SiteTreeFileExtension.php +++ b/code/model/SiteTreeFileExtension.php @@ -14,9 +14,10 @@ class SiteTreeFileExtension extends DataExtension { ReadonlyField::create( 'BackLinkCount', _t('AssetTableField.BACKLINKCOUNT', 'Used on:'), - $this->BackLinkTracking()->Count() . ' ' . _t('AssetTableField.PAGES', 'page(s)')) - ->addExtraClass('cms-description-toggle') - ->setDescription($this->BackLinkHTMLList()), + $this->BackLinkTracking()->Count() . ' ' . _t('AssetTableField.PAGES', 'page(s)') + ) + ->addExtraClass('cms-description-toggle') + ->setDescription($this->BackLinkHTMLList()), 'LastEdited' ); } @@ -27,19 +28,21 @@ class SiteTreeFileExtension extends DataExtension { * @return String */ public function BackLinkHTMLList() { - $html = '' . _t('SiteTreeFileExtension.BACKLINK_LIST_DESCRIPTION', 'This list shows all pages where the file has been added through a WYSIWYG editor.') . ''; + $html = '' . _t( + 'SiteTreeFileExtension.BACKLINK_LIST_DESCRIPTION', + 'This list shows all pages where the file has been added through a WYSIWYG editor.' + ) . ''; $html .= ''; @@ -54,31 +57,13 @@ class SiteTreeFileExtension extends DataExtension { * @param string $limit * @return ManyManyList */ - public function BackLinkTracking($filter = null, $sort = null, $join = null, $limit = null) { - if($filter !== null || $sort !== null || $join !== null || $limit !== null) { - Deprecation::notice('4.0', 'The $filter, $sort, $join and $limit parameters for - SiteTreeFileExtension::BackLinkTracking() have been deprecated. - Please manipluate the returned list directly.', Deprecation::SCOPE_GLOBAL); - } - + public function BackLinkTracking() { if(class_exists("Subsite")){ $rememberSubsiteFilter = Subsite::$disable_subsite_filter; Subsite::disable_subsite_filter(true); } - - if($filter || $sort || $join || $limit) { - Deprecation::notice('4.0', 'The $filter, $sort, $join and $limit parameters for - SiteTreeFileExtension::BackLinkTracking() have been deprecated. - Please manipluate the returned list directly.', Deprecation::SCOPE_GLOBAL); - } $links = $this->owner->getManyManyComponents('BackLinkTracking'); - if($this->owner->ID) { - $links = $links - ->where($filter) - ->sort($sort) - ->limit($limit); - } $this->owner->extend('updateBackLinkTracking', $links); if(class_exists("Subsite")){ @@ -115,7 +100,9 @@ class SiteTreeFileExtension extends DataExtension { // This will syncLinkTracking on draft Versioned::reading_stage('Stage'); $brokenPages = DataObject::get('SiteTree')->byIDs($brokenPageIDs); - foreach($brokenPages as $brokenPage) $brokenPage->write(); + foreach($brokenPages as $brokenPage) { + $brokenPage->write(); + } // This will syncLinkTracking on published Versioned::reading_stage('Live'); @@ -131,22 +118,23 @@ class SiteTreeFileExtension extends DataExtension { /** * Rewrite links to the $old file to now point to the $new file. * - * @uses SiteTree->rewriteFileURL() - * - * @param String $old File path relative to the webroot - * @param String $new File path relative to the webroot + * @uses SiteTree->rewriteFileID() */ - public function updateLinks($old, $new) { - if(class_exists('Subsite')) Subsite::disable_subsite_filter(true); + public function updateLinks() { + if(class_exists('Subsite')) { + Subsite::disable_subsite_filter(true); + } $pages = $this->owner->BackLinkTracking(); - - $summary = ""; if($pages) { - foreach($pages as $page) $page->rewriteFileURL($old,$new); + foreach($pages as $page) { + $page->rewriteFileLinks(); + } } - if(class_exists('Subsite')) Subsite::disable_subsite_filter(false); + if(class_exists('Subsite')) { + Subsite::disable_subsite_filter(false); + } } } diff --git a/code/model/SiteTreeLinkTracking.php b/code/model/SiteTreeLinkTracking.php index fead777c..eecc3d65 100644 --- a/code/model/SiteTreeLinkTracking.php +++ b/code/model/SiteTreeLinkTracking.php @@ -12,13 +12,13 @@ * referenced in any HTMLText fields, and two booleans to indicate if there are any broken links. Call * augmentSyncLinkTracking to update those fields with any changes to those fields. * - * @property SiteTree owner + * @property SiteTree $owner * - * @property bool HasBrokenFile - * @property bool HasBrokenLink + * @property bool $HasBrokenFile + * @property bool $HasBrokenLink * - * @method ManyManyList LinkTracking List of site pages linked on this page. - * @method ManyManyList ImageTracking List of Images linked on this page. + * @method ManyManyList LinkTracking() List of site pages linked on this page. + * @method ManyManyList ImageTracking() List of Images linked on this page. */ class SiteTreeLinkTracking extends DataExtension { @@ -110,22 +110,25 @@ class SiteTreeLinkTracking extends DataExtension { // Add file tracking for image references if($images = $htmlValue->getElementsByTagName('img')) foreach($images as $img) { - if($image = File::find($path = urldecode(Director::makeRelative($img->getAttribute('src'))))) { + // {@see HtmlEditorField} for data-fileid source + $fileID = $img->getAttribute('data-fileid'); + if(!$fileID) { + continue; + } + + // Assuming a local file is linked, check if it's valid + if($image = File::get()->byID($fileID)) { $linkedFiles[] = $image->ID; } else { - if(substr($path, 0, strlen(ASSETS_DIR) + 1) == ASSETS_DIR . '/') { - $record->HasBrokenFile = true; - } + $record->HasBrokenFile = true; } } // Update the "LinkTracking" many_many - if($record->ID && $record->manyManyComponent('LinkTracking') && $tracker = $record->LinkTracking()) { - $tracker->removeByFilter(sprintf( - '"FieldName" = \'%s\' AND "%s" = %d', - $fieldName, - $tracker->getForeignKey(), - $record->ID + if($record->ID && $record->manyManyComponent('LinkTracking') && ($tracker = $record->LinkTracking())) { + $tracker->removeByFilter(array( + sprintf('"FieldName" = ? AND "%s" = ?', $tracker->getForeignKey()) + => array($fieldName, $record->ID) )); if($linkedPages) foreach($linkedPages as $item) { @@ -134,12 +137,10 @@ class SiteTreeLinkTracking extends DataExtension { } // Update the "ImageTracking" many_many - if($record->ID && $record->manyManyComponent('ImageTracking') && $tracker = $record->ImageTracking()) { - $tracker->removeByFilter(sprintf( - '"FieldName" = \'%s\' AND "%s" = %d', - $fieldName, - $tracker->getForeignKey(), - $record->ID + if($record->ID && $record->manyManyComponent('ImageTracking') && ($tracker = $record->ImageTracking())) { + $tracker->removeByFilter(array( + sprintf('"FieldName" = ? AND "%s" = ?', $tracker->getForeignKey()) + => array($fieldName, $record->ID) )); if($linkedFiles) foreach($linkedFiles as $item) { diff --git a/tasks/FilesystemSyncTask.php b/tasks/FilesystemSyncTask.php deleted file mode 100644 index d5da7f98..00000000 --- a/tasks/FilesystemSyncTask.php +++ /dev/null @@ -1,25 +0,0 @@ -getVar('skipSyncLinkTracking'))) . "\n"; - } - -} diff --git a/tests/behat/features/insert-a-link.feature b/tests/behat/features/insert-a-link.feature index 84015b03..37e58ee8 100644 --- a/tests/behat/features/insert-a-link.feature +++ b/tests/behat/features/insert-a-link.feature @@ -32,7 +32,7 @@ So that I can link to a external website or a page on my site And I fill in the "internal" dropdown with "Details" And I wait for 1 second And I select "youranchor" from "Form_EditorToolbarLinkForm_AnchorSelector" - And I press the "Insert link" button + And I press the "Insert" button Then the "Content" HTML field should contain "awesome" # Required to avoid "unsaved changes" browser dialog Then I press the "Save draft" button @@ -53,8 +53,8 @@ So that I can link to a external website or a page on my site When I press the "Insert Link" button When I select the "Download a file" radio button And I attach the file "testfile.jpg" to "file[Uploads][]" with HTML5 - And I press the "Insert link" button - Then the "Content" HTML field should contain "awesome" + And I press the "Insert" button + Then the "Content" HTML field should contain "awesome" # Required to avoid "unsaved changes" browser dialog Then I press the "Save draft" button @@ -64,7 +64,7 @@ So that I can link to a external website or a page on my site When I press the "Insert Link" button When I select the "Anchor on this page" radio button And I select "myanchor" from "Form_EditorToolbarLinkForm_AnchorSelector" - And I press the "Insert link" button + And I press the "Insert" button Then the "Content" HTML field should contain "awesome" # Required to avoid "unsaved changes" browser dialog Then I press the "Save draft" button @@ -77,7 +77,7 @@ So that I can link to a external website or a page on my site Then the "Form_EditorToolbarLinkForm_external" field should contain "http://silverstripe.org" # This doesn't seem to suffer from that issue When I fill in "http://google.com" for "URL" - And I press the "Insert link" button + And I press the "Insert" button Then the "Content" HTML field should contain "awesome" # Required to avoid "unsaved changes" browser dialog Then I press the "Save draft" button diff --git a/tests/behat/features/insert-an-image.feature b/tests/behat/features/insert-an-image.feature index cbad817a..c3bbd429 100644 --- a/tests/behat/features/insert-an-image.feature +++ b/tests/behat/features/insert-an-image.feature @@ -18,7 +18,7 @@ Feature: Insert an image into a page When I click "add by URL" in the ".ss-uploadfield-item-info" element And I fill in "RemoteURL" with "http://www.silverstripe.org/themes/ssv3/img/ss_logo.png" And I press the "Add url" button - Then I should see "ss_logo.png (www.silverstripe.org)" in the ".ss-assetuploadfield span.name" element + Then I should see "ss_logo.png" in the ".ss-assetuploadfield span.name" element When I press the "Insert" button Then the "Content" HTML field should contain "ss_logo.png" @@ -31,9 +31,9 @@ Feature: Insert an image into a page And I attach the file "testfile.jpg" to "AssetUploadField" with HTML5 # TODO Delay previous step until upload succeeded And I wait for 2 seconds - Then there should be a file "assets/Uploads/testfile.jpg" + Then there should be a file "assets/Uploads/59de0c841f/testfile.jpg" When I press the "Insert" button - Then the "Content" HTML field should contain "testfile.jpg" + Then the "Content" HTML field should contain "testfile__Resampled.jpg" # Required to avoid "unsaved changed" browser dialog Then I press the "Save draft" button @@ -45,10 +45,10 @@ Feature: Insert an image into a page # TODO Delay previous step until upload succeeded And I wait for 2 seconds # Note change in default behaviour from 3.1, respect default Upload.replaceFile=false - Then there should be a file "assets/Uploads/file1.jpg" - And there should be a file "assets/Uploads/file1-v2.jpg" + Then there should be a file "assets/Uploads/3d0ef6ec37/file1.jpg" + And there should be a file "assets/Uploads/3d0ef6ec37/file1-v2.jpg" When I press the "Insert" button - Then the "Content" HTML field should contain "file1-v2.jpg" + Then the "Content" HTML field should contain "file1-v2__Resampled.jpg" # Required to avoid "unsaved changed" browser dialog Then I press the "Save draft" button @@ -57,7 +57,7 @@ Feature: Insert an image into a page And I fill in the "ParentID" dropdown with "folder1" And I click on "file1" in the "Files" table When I press the "Insert" button - Then the "Content" HTML field should contain "file1.jpg" + Then the "Content" HTML field should contain "file1__Resampled.jpg" # Required to avoid "unsaved changed" browser dialog Then I press the "Save draft" button @@ -68,7 +68,7 @@ Feature: Insert an image into a page And I press the "Edit" button When I fill in "Alternative text (alt)" with "My alt" And I press the "Insert" button - Then the "Content" HTML field should contain "file1.jpg" + Then the "Content" HTML field should contain "file1__Resampled.jpg" And the "Content" HTML field should contain "My alt" # Required to avoid "unsaved changed" browser dialog Then I press the "Save draft" button @@ -76,14 +76,14 @@ Feature: Insert an image into a page # TODO This needs to support using drag handles, as we no longer have 'Width' or 'Height' input fields @todo Scenario: I can edit dimensions of an existing image - Given the "page" "About us" contains "" + Given the "page" "About us" contains "" And I reload the current page - When I highlight "" in the "Content" HTML field + When I highlight "" in the "Content" HTML field And I press the "Insert Media" button Then I should see "file1.jpg" When I fill in "Width" with "10" When I fill in "Height" with "20" And I press the "Insert" button - Then the "Content" HTML field should contain "" + Then the "Content" HTML field should contain "" # Required to avoid "unsaved changed" browser dialog Then I press the "Save draft" button diff --git a/tests/behat/features/manage-files.feature b/tests/behat/features/manage-files.feature index 20a8abb6..0bb33677 100644 --- a/tests/behat/features/manage-files.feature +++ b/tests/behat/features/manage-files.feature @@ -6,7 +6,7 @@ Feature: Manage files Background: Given a "image" "assets/folder1/file1.jpg" was created "2012-01-01 12:00:00" - And a "image" "assets/folder1/folder1.1/file2.jpg" was created "2010-01-01 12:00:00" + And a "image" "assets/folder1/folder1-1/file2.jpg" was created "2010-01-01 12:00:00" And a "folder" "assets/folder2" And I am logged in with "ADMIN" permissions And I go to "/admin/assets" @@ -21,7 +21,7 @@ Feature: Manage files Scenario: I can list files in a folder Given I click on "folder1" in the "Files" table Then the "folder1" table should contain "file1" - And the "folder1" table should not contain "file1.1" + And the "folder1" table should not contain "file1-1" Scenario: I can upload a file to a folder Given I click on "folder1" in the "Files" table diff --git a/tests/model/FileLinkTrackingTest.php b/tests/model/FileLinkTrackingTest.php index 75c117bc..22296bdb 100644 --- a/tests/model/FileLinkTrackingTest.php +++ b/tests/model/FileLinkTrackingTest.php @@ -8,40 +8,52 @@ class FileLinkTrackingTest extends SapphireTest { public function setUp() { parent::setUp(); + AssetStoreTest_SpyStore::activate('FileLinkTrackingTest'); $this->logInWithPermission('ADMIN'); - - if(!file_exists(ASSETS_PATH)) mkdir(ASSETS_PATH); - $fh = fopen(ASSETS_PATH . '/testscript-test-file.pdf', "w"); - fwrite($fh, str_repeat('x',1000000)); - fclose($fh); + + // Write file contents + $files = File::get()->exclude('ClassName', 'Folder'); + foreach($files as $file) { + $destPath = AssetStoreTest_SpyStore::getLocalPath($file); + Filesystem::makeFolder(dirname($destPath)); + file_put_contents($destPath, str_repeat('x', 1000000)); + } + + // Since we can't hard-code IDs, manually inject image tracking shortcode + $imageID = $this->idFromFixture('Image', 'file1'); + $page = $this->objFromFixture('Page', 'page1'); + $page->Content = sprintf( + '

', + $imageID + ); + $page->write(); } public function tearDown() { + AssetStoreTest_SpyStore::reset(); parent::tearDown(); - $testFiles = array( - '/testscript-test-file.pdf', - '/renamed-test-file.pdf', - '/renamed-test-file-second-time.pdf', - ); - foreach($testFiles as $file) { - if(file_exists(ASSETS_PATH . $file)) unlink(ASSETS_PATH . $file); - } } public function testFileRenameUpdatesDraftAndPublishedPages() { $page = $this->objFromFixture('Page', 'page1'); $this->assertTrue($page->doPublish()); - $this->assertContains('ID))->value()); + $this->assertContains( + 'ID))->value() + ); - $file = $this->objFromFixture('File', 'file1'); - $file->Name = 'renamed-test-file.pdf'; + $file = $this->objFromFixture('Image', 'file1'); + $file->Name = 'renamed-test-file.jpg'; $file->write(); - $this->assertContains('ID))->value()); - $this->assertContains('ID))->value()); + $this->assertContains( + 'ID))->value() + ); + $this->assertContains( + 'ID))->value() + ); } public function testFileLinkRewritingOnVirtualPages() { @@ -56,15 +68,19 @@ class FileLinkTrackingTest extends SapphireTest { $svp->doPublish(); // Rename the file - $file = $this->objFromFixture('File', 'file1'); - $file->Name = 'renamed-test-file.pdf'; + $file = $this->objFromFixture('Image', 'file1'); + $file->Name = 'renamed-test-file.jpg'; $file->write(); // Verify that the draft and publish virtual pages both have the corrected link - $this->assertContains('ID))->value()); - $this->assertContains('ID))->value()); + $this->assertContains( + 'ID))->value() + ); + $this->assertContains( + 'ID))->value() + ); } public function testLinkRewritingOnAPublishedPageDoesntMakeItEditedOnDraft() { @@ -74,8 +90,8 @@ class FileLinkTrackingTest extends SapphireTest { $this->assertFalse($page->getIsModifiedOnStage()); // Rename the file - $file = $this->objFromFixture('File', 'file1'); - $file->Name = 'renamed-test-file.pdf'; + $file = $this->objFromFixture('Image', 'file1'); + $file->Name = 'renamed-test-file.jpg'; $file->write(); // Caching hack @@ -89,25 +105,30 @@ class FileLinkTrackingTest extends SapphireTest { public function testTwoFileRenamesInARowWork() { $page = $this->objFromFixture('Page', 'page1'); $this->assertTrue($page->doPublish()); - $this->assertContains('assertContains( + 'ID))->value()); // Rename the file twice - $file = $this->objFromFixture('File', 'file1'); - $file->Name = 'renamed-test-file.pdf'; + $file = $this->objFromFixture('Image', 'file1'); + $file->Name = 'renamed-test-file.jpg'; $file->write(); // TODO Workaround for bug in DataObject->getChangedFields(), which returns stale data, // and influences File->updateFilesystem() $file = DataObject::get_by_id('File', $file->ID); - $file->Name = 'renamed-test-file-second-time.pdf'; + $file->Name = 'renamed-test-file-second-time.jpg'; $file->write(); // Confirm that the correct image is shown in both the draft and live site - $this->assertContains('ID))->value()); - $this->assertContains('ID))->value()); + $this->assertContains( + 'ID))->value() + ); + $this->assertContains( + 'ID))->value() + ); } } diff --git a/tests/model/FileLinkTrackingTest.yml b/tests/model/FileLinkTrackingTest.yml index 45906aea..c441a251 100644 --- a/tests/model/FileLinkTrackingTest.yml +++ b/tests/model/FileLinkTrackingTest.yml @@ -1,10 +1,12 @@ # These need to come first so that SiteTree has the link meta-data written. -File: - file1: - Filename: assets/testscript-test-file.pdf +Image: + file1: + FileFilename: testscript-test-file.jpg + FileHash: 55b443b60176235ef09801153cca4e6da7494a0c + Name: testscript-test-file.jpg Page: - page1: - Title: page1 - URLSegment: page1 - Content:

\ No newline at end of file + page1: + Title: page1 + URLSegment: page1 + Content: '

' diff --git a/tests/model/SiteTreeHTMLEditorFieldTest.php b/tests/model/SiteTreeHTMLEditorFieldTest.php index c3a63a5b..91530d1a 100644 --- a/tests/model/SiteTreeHTMLEditorFieldTest.php +++ b/tests/model/SiteTreeHTMLEditorFieldTest.php @@ -4,6 +4,25 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest { protected static $use_draft_site = true; + public function setUp() { + parent::setUp(); + AssetStoreTest_SpyStore::activate('SiteTreeHtmlEditorFieldTest'); + $this->logInWithPermission('ADMIN'); + + // Write file contents + $files = File::get()->exclude('ClassName', 'Folder'); + foreach($files as $file) { + $destPath = AssetStoreTest_SpyStore::getLocalPath($file); + Filesystem::makeFolder(dirname($destPath)); + file_put_contents($destPath, str_repeat('x', 1000000)); + } + } + + public function tearDown() { + AssetStoreTest_SpyStore::reset(); + parent::tearDown(); + } + public function testLinkTracking() { $sitetree = $this->objFromFixture('SiteTree', 'home'); $editor = new HtmlEditorField('Content'); @@ -102,21 +121,24 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest { public function testImageTracking() { $sitetree = $this->objFromFixture('SiteTree', 'home'); - $editor = new HtmlEditorField('Content'); - $fileID = $this->idFromFixture('Image', 'example_image'); + $editor = new HtmlEditorField('Content'); + $file = $this->objFromFixture('Image', 'example_image'); - $editor->setValue(''); + $editor->setValue(sprintf('', $file->getURL(), $file->ID)); $editor->saveInto($sitetree); $sitetree->write(); - $this->assertEquals ( - array($fileID => $fileID), $sitetree->ImageTracking()->getIDList(), 'Inserted images are tracked.' + $this->assertEquals( + array($file->ID => $file->ID), + $sitetree->ImageTracking()->getIDList(), + 'Inserted images are tracked.' ); $editor->setValue(null); $editor->saveInto($sitetree); $sitetree->write(); - $this->assertEquals ( - array(), $sitetree->ImageTracking()->getIDList(), 'Tracked images are deleted when removed.' + $this->assertEmpty( + $sitetree->ImageTracking()->getIDList(), + 'Tracked images are deleted when removed.' ); } diff --git a/tests/model/SiteTreeHtmlEditorFieldTest.yml b/tests/model/SiteTreeHtmlEditorFieldTest.yml index b86fc6fb..9855e833 100644 --- a/tests/model/SiteTreeHtmlEditorFieldTest.yml +++ b/tests/model/SiteTreeHtmlEditorFieldTest.yml @@ -8,8 +8,12 @@ SiteTree: File: example_file: + FileFilename: example.pdf + FileHash: 55b443b60176235ef09801153cca4e6da7494a0c Name: example.pdf Image: example_image: + FileFilename: example.jpg + FileHash: 55b443b60176235ef09801153cca4e6da7494a0c Name: example.jpg diff --git a/tests/model/SiteTreeLinkTrackingTest.php b/tests/model/SiteTreeLinkTrackingTest.php index 98ccb71f..54d6ed41 100644 --- a/tests/model/SiteTreeLinkTrackingTest.php +++ b/tests/model/SiteTreeLinkTrackingTest.php @@ -2,7 +2,7 @@ class SiteTreeLinkTrackingTest extends SapphireTest { - function isBroken($content) { + protected function isBroken($content) { $parser = new SiteTreeLinkTracking_Parser(); $htmlValue = Injector::inst()->create('HTMLValue', $content); $links = $parser->process($htmlValue); @@ -11,7 +11,7 @@ class SiteTreeLinkTrackingTest extends SapphireTest { return $links[0]['Broken']; } - function testParser() { + public function testParser() { $this->assertTrue($this->isBroken('link')); $this->assertTrue($this->isBroken('link')); $this->assertTrue($this->isBroken('link')); @@ -34,14 +34,14 @@ class SiteTreeLinkTrackingTest extends SapphireTest { $this->assertFalse($this->isBroken("ID]\">link")); } - function highlight($content) { + protected function highlight($content) { $page = new Page(); $page->Content = $content; $page->write(); return $page->Content; } - function testHighlighter() { + public function testHighlighter() { $content = $this->highlight('link'); $this->assertEquals(substr_count($content, 'ss-broken'), 1, 'A ss-broken class is added to the broken link.'); $this->assertEquals(substr_count($content, 'existing-class'), 1, 'Existing class is not removed.'); @@ -60,4 +60,16 @@ class SiteTreeLinkTrackingTest extends SapphireTest { $this->assertEquals(substr_count($content, 'existing-class'), 1, 'Existing class is not removed.'); } + public function testHasBrokenFile() { + $this->assertTrue($this->pageIsBrokenFile('')); + $this->assertFalse($this->pageIsBrokenFile('')); + } + + protected function pageIsBrokenFile($content) { + $page = new Page(); + $page->Content = $content; + $page->write(); + return $page->HasBrokenFile; + } + }