From 510c55673972ea5aeaca40bc6ef9166fb2b339c3 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 26 Jan 2016 16:56:07 +1300 Subject: [PATCH] API File has Versioned extension API Improved support for versioned DataObject API GridField extensions for versioned dataobjects API De-couple File and ErrorPage API File::handle_shortcode now respects canView() API AssetControlExtension is now able to delete, publish, or protect files API Upload now protects new assets by default --- _config/versioning.yml | 6 + control/RequestHandler.php | 1 + dev/SapphireTest.php | 12 + .../14_Files/03_File_Security.md | 20 +- docs/en/04_Changelogs/4.0.0.md | 32 +- filesystem/AssetControlExtension.php | 364 +++++++++++++----- filesystem/AssetManipulationList.php | 175 +++++++++ filesystem/File.php | 84 ++-- filesystem/FileMigrationHelper.php | 6 +- filesystem/Upload.php | 52 ++- filesystem/storage/DBFile.php | 10 +- forms/FieldList.php | 3 + forms/UploadField.php | 6 + forms/gridfield/GridFieldDetailForm.php | 239 ++++++++---- model/DataExtension.php | 2 + model/{ => versioning}/Versioned.php | 320 ++++++++++++--- .../VersionedGridFieldDetailForm.php | 13 + .../VersionedGridFieldItemRequest.php | 196 ++++++++++ parsers/ShortcodeHandler.php | 7 + tasks/MigrateFileTask.php | 2 + .../filesystem/AssetControlExtensionTest.php | 119 +++++- .../filesystem/AssetManipulationListTest.php | 79 ++++ tests/filesystem/FileMigrationHelperTest.php | 2 + tests/filesystem/FileTest.php | 124 ++++-- tests/filesystem/FolderTest.php | 7 +- tests/filesystem/UploadTest.php | 5 +- tests/forms/uploadfield/AssetFieldTest.php | 11 +- tests/forms/uploadfield/UploadFieldTest.php | 90 ++--- tests/model/VersionedTest.php | 16 + 29 files changed, 1647 insertions(+), 356 deletions(-) create mode 100644 _config/versioning.yml create mode 100644 filesystem/AssetManipulationList.php rename model/{ => versioning}/Versioned.php (87%) create mode 100644 model/versioning/VersionedGridFieldDetailForm.php create mode 100644 model/versioning/VersionedGridFieldItemRequest.php create mode 100644 tests/filesystem/AssetManipulationListTest.php diff --git a/_config/versioning.yml b/_config/versioning.yml new file mode 100644 index 000000000..b028ed285 --- /dev/null +++ b/_config/versioning.yml @@ -0,0 +1,6 @@ +--- +Name: versioning +--- +GridFieldDetailForm: + extensions: + - VersionedGridFieldDetailForm diff --git a/control/RequestHandler.php b/control/RequestHandler.php index 0b4ca2a84..df6552180 100644 --- a/control/RequestHandler.php +++ b/control/RequestHandler.php @@ -460,6 +460,7 @@ class RequestHandler extends ViewableData { * @param int $errorCode * @param string $errorMessage Plaintext error message * @uses SS_HTTPResponse_Exception + * @throws SS_HTTPResponse_Exception */ public function httpError($errorCode, $errorMessage = null) { diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index 920e84127..4751294f2 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -169,12 +169,21 @@ class SapphireTest extends PHPUnit_Framework_TestCase { protected $model; + /** + * State of Versioned before this test is run + * + * @var string + */ + protected $originalReadingMode = null; + public function setUp() { //nest config and injector for each test so they are effectively sandboxed per test Config::nest(); Injector::nest(); + $this->originalReadingMode = \Versioned::get_reading_mode(); + // We cannot run the tests on this abstract class. if(get_class($this) == "SapphireTest") $this->skipTest = true; @@ -525,6 +534,9 @@ class SapphireTest extends PHPUnit_Framework_TestCase { $controller->response->setStatusCode(200); $controller->response->removeHeader('Location'); } + + \Versioned::set_reading_mode($this->originalReadingMode); + //unnest injector / config now that tests are over Injector::unnest(); Config::unnest(); diff --git a/docs/en/02_Developer_Guides/14_Files/03_File_Security.md b/docs/en/02_Developer_Guides/14_Files/03_File_Security.md index f4687fb60..2da883bf2 100644 --- a/docs/en/02_Developer_Guides/14_Files/03_File_Security.md +++ b/docs/en/02_Developer_Guides/14_Files/03_File_Security.md @@ -278,23 +278,27 @@ You can customise this with the below config: ### Configuring: Archive behaviour By default, the default extension `AssetControlExtension` will control the disposal of assets -attached to objects when those objects are deleted. For example, unpublished versioned objects -will automatically have their attached assets moved to the protected store. The deletion of -draft or unversioned objects will have those assets permanantly deleted (along with all variants). +attached to objects when those objects are archived. For example, unpublished versioned objects +will automatically have their attached assets moved to the protected store. The archive of +draft or (or deletion of unversioned objects) will have those assets permanantly deleted +(along with all variants). -In some cases, it may be preferable to have any deleted assets archived for versioned dataobjects, -rather than deleted. This uses more disk storage, but will allow the full recovery of archived +Note that regardless of this setting, the database record will still be archived in the +version history for all Versioned DataObjects. + +In some cases, it may be preferable to have any assets retained for archived versioned dataobjects, +instead of deleting them. This uses more disk storage, but will allow the full recovery of archived records and files. -This can be applied to DataObjects on a case by case basis by setting the `archive_assets` +This can be applied to DataObjects on a case by case basis by setting the `keep_archived_assets` config to true on that class. Note that this feature only works with dataobjects with the `Versioned` extension. :::php class MyVersiondObject extends DataObject { - /** Enable archiving */ - private static $archive_assets = true; + /** Ensure assets are archived along with the DataObject */ + private static $keep_archived_assets = true; /** Versioned */ private static $extensions = array('Versioned'); } diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 7af1578b2..8eeb99a66 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -25,6 +25,9 @@ more information. * `Object::useCustomClass` has been removed. You should use the config API with Injector instead. * Upgrade of TinyMCE to version 4. + * `File` is now versioned, and should be published before they can be used on the frontend. + See section on [Migrating File DataObject from 3.x to 4.0](#migrating-file-dataobject-from-3x-to-40) + below for upgrade notes. ## New API @@ -48,6 +51,12 @@ TinyMCE editor. * `HtmlEditorField::setEditorConfig` may now take an instance of a `HtmlEditorConfig` class, as well as a standard config identifier name. + * A lot of standard versioned API has been refactored from `SiteTree` into `Versioned` extension. Now + all versioned DataObjects have canPublish(), canArchive(), canUnpublish(), doPublish(), doArchive() + doUnpublish(), isPublished() and isonDraft() out of the box. However, do*() methods will no longer + automatically check can*() permissions, and must be done by usercode before invocation. + * GridField edit form now has improved support for versioned DataObjects, with basic publishing + actions available when editing records. ### Front-end build tooling for CMS interface @@ -243,6 +252,13 @@ large amounts of memory and run for an extended time. migrate_legacy_file: true +This task will also support migration of existing File DataObjects to file versioning. Any +pre-existing File DataObjects will be automatically published to the live stage, to ensure +that previously visible assets remain visible to the public site. + +If additional security or visibility rules should be applied to File dataobjects, then +make sure to correctly extend `canView` via extensions. + ### Upgrade code which acts on `Image` As all image-specific manipulations has been refactored from `Image` into an `ImageManipulations` trait, which @@ -321,9 +337,11 @@ After: :::php function importTempFile($tmp) { + Versioned::reading_stage('Stage'); $file = new File(); $file->setFromLocalFile($tmp, 'imported/'.basename($tmp)); $file->write(); + $file->doPublish(); } @@ -333,13 +351,19 @@ stored in the assets folder. There are other important considerations in working with File dataobjects which differ from legacy: -* Deleting File dataobjects no longer removes the physical file directly. This is because any file could be referenced - from DBFile fields, and deleting these could be a potentially unsafe operation. * File synchronisation is no longer automatic. This is due to the fact that there is no longer a 1-to-1 relationship between physical files and File dataobjects. -* Moving files now performs a file copy rather than moving the underlying file, although only a single DataObject - will exist, and will reference the destination path. * Folder dataobjects are now purely logical dataobjects, and perform no actual filesystem folder creation on write. +* All Files are versioned, which means that by default, new File records will not be visibile + to the public site. You will need to make sure to invoke ->doPublish() on any File dataobject + you wish visitors to be able to see. + +You can disable File versioning by adding the following to your _config.php + + + :::php + File::remove_extension('Versioned'); + ### Upgrading code performs custom image manipulations diff --git a/filesystem/AssetControlExtension.php b/filesystem/AssetControlExtension.php index 634987590..de771d2fc 100644 --- a/filesystem/AssetControlExtension.php +++ b/filesystem/AssetControlExtension.php @@ -4,110 +4,294 @@ namespace SilverStripe\Filesystem; use DataObject; use Injector; +use Member; +use Versioned; use SilverStripe\Filesystem\Storage\AssetStore; /** - * Provides an interaction mechanism between objects and linked asset references. + * This class provides the necessary business logic to ensure that any assets attached + * to a record are safely deleted, published, or protected during certain operations. + * + * This class will respect the canView() of each object, and will use it to determine + * whether or not public users can access attached assets. Public and live records + * will have their assets promoted to the public store. + * + * Assets which exist only on non-live stages will be protected. + * + * Assets which are no longer referenced will be flushed via explicit delete calls + * to the underlying filesystem. + * + * @property DataObject|Versioned $owner A {@see DataObject}, potentially decorated with {@see Versioned} extension. */ -class AssetControlExtension extends \DataExtension { +class AssetControlExtension extends \DataExtension +{ - /** - * When archiving versioned dataobjects, should assets be archived with them? - * If false, assets will be deleted when the object is removed from draft. - * If true, assets will be instead moved to the protected store. - * - * @var bool - */ - private static $archive_assets = false; + /** + * When archiving versioned dataobjects, should assets be archived with them? + * If false, assets will be deleted when the dataobject is archived. + * If true, assets will be instead moved to the protected store, and can be + * restored when the dataobject is restored from archive. + * + * Note that this does not affect the archiving of the actual database record in any way, + * only the physical file. + * + * Unversioned dataobjects will ignore this option and always delete attached + * assets on deletion. + * + * @config + * @var bool + */ + private static $keep_archived_assets = false; - public function onAfterDelete() { - $assets = $this->findAssets($this->owner); + /** + * Ensure that deletes records remove their underlying file assets, without affecting + * other staged records. + */ + public function onAfterDelete() + { + // Prepare blank manipulation + $manipulations = new AssetManipulationList(); - // When deleting from live, just secure assets - // Note that DataObject::delete() ignores sourceQueryParams - if($this->isVersioned() && \Versioned::current_stage() === \Versioned::get_live_stage()) { - $this->protectAll($assets); - return; - } + // Add all assets for deletion + $this->addAssetsFromRecord($manipulations, $this->owner, AssetManipulationList::STATE_DELETED); - // When deleting from stage then check if we should archive assets - $archive = $this->owner->config()->archive_assets; - if($archive && $this->isVersioned()) { - // Archived assets are kept protected - $this->protectAll($assets); - } else { - // Otherwise remove all assets - $this->deleteAll($assets); - } - } + // Whitelist assets that exist in other stages + $this->addAssetsFromOtherStages($manipulations); - /** - * Return a list of all tuples attached to this dataobject - * Note: Variants are excluded - * - * @param DataObject $record to search - * @return array - */ - protected function findAssets(DataObject $record) { - // Search for dbfile instances - $files = array(); - foreach($record->db() as $field => $db) { - // Extract assets from this database field - list($dbClass) = explode('(', $db); - if(!is_a($dbClass, 'DBFile', true)) { - continue; - } + // Apply visibility rules based on the final manipulation + $this->processManipulation($manipulations); + } - // Omit variant and merge with set - $next = $record->dbObject($field)->getValue(); - unset($next['Variant']); - if ($next) { - $files[] = $next; - } - } + /** + * Ensure that changes to records flush overwritten files, and update the visibility + * of other assets. + */ + public function onBeforeWrite() + { + // Prepare blank manipulation + $manipulations = new AssetManipulationList(); - // De-dupe - return array_map("unserialize", array_unique(array_map("serialize", $files))); - } + // Mark overwritten object as deleted + if($this->owner->isInDB()) { + $priorRecord = DataObject::get(get_class($this->owner))->byID($this->owner->ID); + if($priorRecord) { + $this->addAssetsFromRecord($manipulations, $priorRecord, AssetManipulationList::STATE_DELETED); + } + } - /** - * Determine if versioning rules should be applied to this object - * - * @return bool - */ - protected function isVersioned() { - return $this->owner->has_extension('Versioned'); - } + // Add assets from new record with the correct visibility rules + $state = $this->getRecordState($this->owner); + $this->addAssetsFromRecord($manipulations, $this->owner, $state); - /** - * Delete all assets in the tuple list - * - * @param array $assets - */ - protected function deleteAll($assets) { - $store = $this->getAssetStore(); - foreach($assets as $asset) { - $store->delete($asset['Filename'], $asset['Hash']); - } - } + // Whitelist assets that exist in other stages + $this->addAssetsFromOtherStages($manipulations); - /** - * Move all assets in the list to the protected store - * - * @param array $assets - */ - protected function protectAll($assets) { - $store = $this->getAssetStore(); - foreach($assets as $asset) { - $store->protect($asset['Filename'], $asset['Hash']); - } - } + // Apply visibility rules based on the final manipulation + $this->processManipulation($manipulations); + } - /** - * @return AssetStore - */ - protected function getAssetStore() { - return Injector::inst()->get('AssetStore'); - } + /** + * Check default state of this record + * + * @param DataObject $record + * @return string One of AssetManipulationList::STATE_* constants + */ + protected function getRecordState($record) { + if($this->isVersioned()) { + // Check stage this record belongs to + $stage = $record->getSourceQueryParam('Versioned.stage') ?: Versioned::current_stage(); + // Non-live stages are automatically non-public + if($stage !== Versioned::get_live_stage()) { + return AssetManipulationList::STATE_PROTECTED; + } + } + + // Check if canView permits anonymous viewers + return $record->canView(Member::create()) + ? AssetManipulationList::STATE_PUBLIC + : AssetManipulationList::STATE_PROTECTED; + } + + /** + * Given a set of asset manipulations, trigger any necessary publish, protect, or + * delete actions on each asset. + * + * @param AssetManipulationList $manipulations + */ + protected function processManipulation(AssetManipulationList $manipulations) + { + // When deleting from stage then check if we should archive assets + $archive = $this->owner->config()->keep_archived_assets; + // Publish assets + $this->publishAll($manipulations->getPublicAssets()); + + // Protect assets + $this->protectAll($manipulations->getProtectedAssets()); + + // Check deletion policy + $deletedAssets = $manipulations->getDeletedAssets(); + if ($archive && $this->isVersioned()) { + // Archived assets are kept protected + $this->protectAll($deletedAssets); + } else { + // Otherwise remove all assets + $this->deleteAll($deletedAssets); + } + } + + /** + * Checks all stages other than the current stage, and check the visibility + * of assets attached to those records. + * + * @param AssetManipulationList $manipulation Set of manipulations to add assets to + */ + protected function addAssetsFromOtherStages(AssetManipulationList $manipulation) + { + // Skip unversioned or unsaved assets + if(!$this->isVersioned() || !$this->owner->isInDB()) { + return; + } + + // Unauthenticated member to use for checking visibility + $baseClass = \ClassInfo::baseDataClass($this->owner); + $filter = array("\"{$baseClass}\".\"ID\"" => $this->owner->ID); + $stages = $this->owner->getVersionedStages(); // {@see Versioned::getVersionedStages} + foreach ($stages as $stage) { + // Skip current stage; These should be handled explicitly + if($stage === Versioned::current_stage()) { + continue; + } + + // Check if record exists in this stage + $record = Versioned::get_one_by_stage($baseClass, $stage, $filter); + if (!$record) { + continue; + } + + // Check visibility of this record, and record all attached assets + $state = $this->getRecordState($record); + $this->addAssetsFromRecord($manipulation, $record, $state); + } + } + + /** + * Given a record, add all assets it contains to the given manipulation. + * State can be declared for this record, otherwise the underlying DataObject + * will be queried for canView() to see if those assets are public + * + * @param AssetManipulationList $manipulation Set of manipulations to add assets to + * @param DataObject $record Record + * @param string $state One of AssetManipulationList::STATE_* constant values. + */ + protected function addAssetsFromRecord(AssetManipulationList $manipulation, DataObject $record, $state) + { + // Find all assets attached to this record + $assets = $this->findAssets($record); + if (empty($assets)) { + return; + } + + // Add all assets to this stage + foreach ($assets as $asset) { + $manipulation->addAsset($asset, $state); + } + } + + /** + * Return a list of all tuples attached to this dataobject + * Note: Variants are excluded + * + * @param DataObject $record to search + * @return array + */ + protected function findAssets(DataObject $record) + { + // Search for dbfile instances + $files = array(); + foreach ($record->db() as $field => $db) { + // Extract assets from this database field + list($dbClass) = explode('(', $db); + if (!is_a($dbClass, 'DBFile', true)) { + continue; + } + + // Omit variant and merge with set + $next = $record->dbObject($field)->getValue(); + unset($next['Variant']); + if ($next) { + $files[] = $next; + } + } + + // De-dupe + return array_map("unserialize", array_unique(array_map("serialize", $files))); + } + + /** + * Determine if {@see Versioned) extension rules should be applied to this object + * + * @return bool + */ + protected function isVersioned() + { + return $this->owner->has_extension('Versioned') && class_exists('Versioned'); + } + + /** + * Delete all assets in the tuple list + * + * @param array $assets + */ + protected function deleteAll($assets) + { + if (empty($assets)) { + return; + } + $store = $this->getAssetStore(); + foreach ($assets as $asset) { + $store->delete($asset['Filename'], $asset['Hash']); + } + } + + /** + * Move all assets in the list to the public store + * + * @param array $assets + */ + protected function publishAll($assets) + { + if (empty($assets)) { + return; + } + + $store = $this->getAssetStore(); + foreach ($assets as $asset) { + $store->publish($asset['Filename'], $asset['Hash']); + } + } + + /** + * Move all assets in the list to the protected store + * + * @param array $assets + */ + protected function protectAll($assets) + { + if (empty($assets)) { + return; + } + $store = $this->getAssetStore(); + foreach ($assets as $asset) { + $store->protect($asset['Filename'], $asset['Hash']); + } + } + + /** + * @return AssetStore + */ + protected function getAssetStore() + { + return Injector::inst()->get('AssetStore'); + } } diff --git a/filesystem/AssetManipulationList.php b/filesystem/AssetManipulationList.php new file mode 100644 index 000000000..c8ded2fec --- /dev/null +++ b/filesystem/AssetManipulationList.php @@ -0,0 +1,175 @@ +addPublicAsset($asset); + case self::STATE_PROTECTED: + return $this->addProtectedAsset($asset); + case self::STATE_DELETED: + return $this->addDeletedAsset($asset); + default: + throw new \InvalidArgumentException("Invalid state {$state}"); + } + } + + /** + * Mark a file as public + * + * @param array $asset Asset tuple + * @return bool True if the asset was added to the public set + */ + public function addPublicAsset($asset) + { + // Remove from protected / deleted lists + $key = $this->getAssetKey($asset); + unset($this->protected[$key]); + unset($this->deleted[$key]); + // Skip if already public + if(isset($this->public[$key])) { + return false; + } + unset($asset['Variant']); + $this->public[$key] = $asset; + return true; + } + + /** + * Record an asset as protected + * + * @param array $asset Asset tuple + * @return bool True if the asset was added to the protected set + */ + public function addProtectedAsset($asset) + { + $key = $this->getAssetKey($asset); + // Don't demote from public + if (isset($this->public[$key])) { + return false; + } + unset($this->deleted[$key]); + // Skip if already protected + if(isset($this->protected[$key])) { + return false; + } + unset($asset['Variant']); + $this->protected[$key] = $asset; + return true; + } + + /** + * Record an asset as deleted + * + * @param array $asset Asset tuple + * @return bool True if the asset was added to the deleted set + */ + public function addDeletedAsset($asset) + { + $key = $this->getAssetKey($asset); + // Only delete if this doesn't exist in any non-deleted state + if (isset($this->public[$key]) || isset($this->protected[$key])) { + return false; + } + // Skip if already deleted + if(isset($this->deleted[$key])) { + return false; + } + unset($asset['Variant']); + $this->deleted[$key] = $asset; + return true; + } + + /** + * Get all public assets + * + * @return array + */ + public function getPublicAssets() + { + return $this->public; + } + + /** + * Get protected assets + * + * @return array + */ + public function getProtectedAssets() + { + return $this->protected; + } + + /** + * Get deleted assets + * + * @return array + */ + public function getDeletedAssets() + { + return $this->deleted; + } +} diff --git a/filesystem/File.php b/filesystem/File.php index dd69d3854..20b4055a8 100644 --- a/filesystem/File.php +++ b/filesystem/File.php @@ -61,16 +61,27 @@ use SilverStripe\Filesystem\Storage\AssetStore; * * @method File Parent() Returns parent File * @method Member Owner() Returns Member object of file owner. + * + * @mixin Hierarchy + * @mixin Versioned */ class File extends DataObject implements ShortcodeHandler, AssetContainer { use ImageManipulation; - private static $default_sort ="\"Name\""; + private static $default_sort = "\"Name\""; - private static $singular_name ="File"; + private static $singular_name = "File"; - private static $plural_name ="Files"; + private static $plural_name = "Files"; + + /** + * Permissions necessary to view files outside of the live stage (e.g. archive / draft stage). + * + * @config + * @var array + */ + private static $non_live_permissions = array('CMS_ACCESS_LeftAndMain', 'CMS_ACCESS_AssetAdmin', 'VIEW_DRAFT_CONTENT'); private static $db = array( "Name" =>"Varchar(255)", @@ -81,8 +92,8 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { ); private static $has_one = array( - "Parent" =>"File", - "Owner" =>"Member" + "Parent" => "File", + "Owner" => "Member" ); private static $defaults = array( @@ -91,6 +102,7 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { private static $extensions = array( "Hierarchy", + "Versioned" ); private static $casting = array( @@ -202,18 +214,30 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { * @return string Result of the handled shortcode */ public static function handle_shortcode($arguments, $content, $parser, $shortcode, $extra = array()) { - if(!isset($arguments['id']) || !is_numeric($arguments['id'])) return; + if(!isset($arguments['id']) || !is_numeric($arguments['id'])) { + return null; + } + /** @var File|SiteTree $record */ $record = DataObject::get_by_id('File', $arguments['id']); + // Check record for common errors + $errorCode = null; if (!$record) { - if(class_exists('ErrorPage')) { - $record = ErrorPage::get()->filter("ErrorCode", 404)->first(); + $errorCode = 404; + } elseif(!$record->canView()) { + $errorCode = 403; + } + if($errorCode) { + $result = static::singleton()->invokeWithExtensions('getErrorRecordFor', $errorCode); + $result = array_filter($result); + if($result) { + $record = reset($result); } + } - if (!$record) { - return; // There were no suitable matches at all. - } + if (!$record) { + return null; // There were no suitable matches at all. } // build the HTML tag @@ -305,10 +329,8 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { } /** - * @todo Enforce on filesystem URL level via mod_rewrite - * * @param Member $member - * @return boolean + * @return bool */ public function canView($member = null) { if(!$member) { @@ -401,7 +423,7 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { ReadonlyField::create( 'ClickableURL', _t('AssetTableField.URL','URL'), - sprintf('%s', $this->Link(), $this->RelativeLink()) + sprintf('%s', $this->Link(), $this->Link()) ) ->setDontEscape(true), new DateField_Disabled("Created", _t('AssetTableField.CREATED','First uploaded') . ':'), @@ -505,8 +527,6 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { * Make sure the file has a name */ protected function onBeforeWrite() { - parent::onBeforeWrite(); - // Set default owner if(!$this->isInDB() && !$this->OwnerID) { $this->OwnerID = Member::currentUserID(); @@ -519,6 +539,8 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { // Propegate changes to the AssetStore and update the DBFile field $this->updateFilesystem(); + + parent::onBeforeWrite(); } /** @@ -562,12 +584,6 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { return true; } - protected function onAfterWrite() { - parent::onAfterWrite(); - // Update any database references - $this->updateLinks(); - } - /** * Collate selected descendants of this page. * $condition will be evaluated on each descendant, and if it is succeeds, that item will be added @@ -642,15 +658,6 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { return $name; } - /** - * Trigger update of all links to this file - * - * If CMS Module is installed, {@see SiteTreeFileExtension::updateLinks} - */ - protected function updateLinks() { - $this->extend('updateLinks'); - } - /** * Gets the URL of this file * @@ -707,6 +714,19 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { return $this->Name; } + /** + * Ensure that parent folders are published before this one is published + * + * @todo Solve this via triggered publishing / ownership in the future + */ + public function onBeforePublish() { + // Relies on Parent() returning the stage record + $parent = $this->Parent(); + if($parent && $parent->exists()) { + $parent->doPublish(); + } + } + /** * Update the ParentID and Name for the given filename. * diff --git a/filesystem/FileMigrationHelper.php b/filesystem/FileMigrationHelper.php index 681eb46ba..fff99f85a 100644 --- a/filesystem/FileMigrationHelper.php +++ b/filesystem/FileMigrationHelper.php @@ -34,6 +34,8 @@ class FileMigrationHelper extends Object { // Loop over all files $count = 0; + $originalState = \Versioned::get_reading_mode(); + \Versioned::reading_stage('Stage'); $filenameMap = $this->getFilenameArray(); foreach($this->getFileQuery() as $file) { // Get the name of the file to import @@ -43,6 +45,7 @@ class FileMigrationHelper extends Object { $count++; } } + \Versioned::set_reading_mode($originalState); return $count; } @@ -73,8 +76,9 @@ class FileMigrationHelper extends Object { $this->setFilename($result['Filename']); } - // Save + // Save and publish $file->write(); + $file->doPublish(); return true; } diff --git a/filesystem/Upload.php b/filesystem/Upload.php index 3058f559d..d0baaa5f8 100644 --- a/filesystem/Upload.php +++ b/filesystem/Upload.php @@ -68,6 +68,13 @@ class Upload extends Controller { */ protected $errors = array(); + /** + * Default visibility to assign uploaded files + * + * @var string + */ + protected $defaultVisibility = AssetStore::VISIBILITY_PROTECTED; + /** * A foldername relative to /assets, * where all uploaded files are stored by default. @@ -198,7 +205,10 @@ class Upload extends Controller { $conflictResolution = $this->replaceFile ? AssetStore::CONFLICT_OVERWRITE : AssetStore::CONFLICT_RENAME; - $config = array('conflict' => $conflictResolution); + $config = array( + 'conflict' => $conflictResolution, + 'visibility' => $this->getDefaultVisibility() + ); return $container->setFromLocalFile($tmpFile['tmp_name'], $filename, null, null, $config); } @@ -210,7 +220,7 @@ class Upload extends Controller { * @param string $folderPath * @return string|false Value of filename tuple, or false if invalid */ - protected function getValidFilename($tmpFile, $folderPath = false) { + protected function getValidFilename($tmpFile, $folderPath = null) { if(!is_array($tmpFile)) { throw new InvalidArgumentException( "Upload::load() Not passed an array. Most likely, the form hasn't got the right enctype" @@ -245,6 +255,7 @@ class Upload extends Controller { * * @param string $filename * @return string $filename A filename safe to write to + * @throws Exception */ protected function resolveExistingFile($filename) { // Create a new file record (or try to retrieve an existing one) @@ -252,7 +263,7 @@ class Upload extends Controller { $fileClass = File::get_class_for_file_extension( File::get_file_extension($filename) ); - $this->file = $fileClass::create(); + $this->file = Object::create($fileClass); } // Skip this step if not writing File dataobjects @@ -288,14 +299,14 @@ class Upload extends Controller { } /** - * @return Boolean + * @param bool $replace */ - public function setReplaceFile($bool) { - $this->replaceFile = $bool; + public function setReplaceFile($replace) { + $this->replaceFile = $replace; } /** - * @return Boolean + * @return bool */ public function getReplaceFile() { return $this->replaceFile; @@ -368,6 +379,28 @@ class Upload extends Controller { return $this->errors; } + /** + * Get default visibility for uploaded files. {@see AssetStore} + * One of the values of AssetStore::VISIBILITY_* constants + * + * @return string + */ + public function getDefaultVisibility() { + return $this->defaultVisibility; + } + + /** + * Assign default visibility for uploaded files. {@see AssetStore} + * One of the values of AssetStore::VISIBILITY_* constants + * + * @param string $visibility + * @return $this + */ + public function setDefaultVisibility($visibility) { + $this->defaultVisibility = $visibility; + return $this; + } + } /** @@ -497,7 +530,6 @@ class Upload_Validator { // make sure all extensions are lowercase $rules = array_change_key_case($rules, CASE_LOWER); $finalRules = array(); - $tmpSize = 0; foreach ($rules as $rule => $value) { if (is_numeric($value)) { @@ -534,7 +566,9 @@ class Upload_Validator { * @param array $rules List of extensions */ public function setAllowedExtensions($rules) { - if(!is_array($rules)) return false; + if(!is_array($rules)) { + return; + } // make sure all rules are lowercase foreach($rules as &$rule) $rule = strtolower($rule); diff --git a/filesystem/storage/DBFile.php b/filesystem/storage/DBFile.php index ddd0e97a3..8ac58ed0b 100644 --- a/filesystem/storage/DBFile.php +++ b/filesystem/storage/DBFile.php @@ -17,7 +17,7 @@ use SilverStripe\Filesystem\Storage\AssetStore; * @package framework * @subpackage filesystem */ -class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandler { +class DBFile extends CompositeDBField implements AssetContainer { use ImageManipulation; @@ -311,14 +311,6 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle ->getStore() ->exists($this->Filename, $this->Hash, $this->Variant); } - - public static function get_shortcodes() { - return 'dbfile_link'; - } - - public static function handle_shortcode($arguments, $content, $parser, $shortcode, $extra = array()) { - // @todo - } public function getFilename() { return $this->getField('Filename'); diff --git a/forms/FieldList.php b/forms/FieldList.php index 83be3f75c..cac3d88bb 100644 --- a/forms/FieldList.php +++ b/forms/FieldList.php @@ -313,6 +313,9 @@ class FieldList extends ArrayList { * You can use dot syntax to get fields from child composite fields * * @todo Implement similarly to dataFieldByName() to support nested sets - or merge with dataFields() + * + * @param string $name + * @return FormField */ public function fieldByName($name) { $name = $this->rewriteTabPath($name); diff --git a/forms/UploadField.php b/forms/UploadField.php index 0fae48268..bb29a1c8e 100644 --- a/forms/UploadField.php +++ b/forms/UploadField.php @@ -1142,6 +1142,12 @@ class UploadField extends FileField { // Search for relations that can hold the uploaded files, but don't fallback // to default if there is no automatic relation if ($relationClass = $this->getRelationAutosetClass(null)) { + // Allow File to be subclassed + if($relationClass === 'File' && isset($tmpFile['name'])) { + $relationClass = File::get_class_for_file_extension( + File::get_file_extension($tmpFile['name']) + ); + } // Create new object explicitly. Otherwise rely on Upload::load to choose the class. $fileObject = Object::create($relationClass); if(! ($fileObject instanceof DataObject) || !($fileObject instanceof AssetContainer)) { diff --git a/forms/gridfield/GridFieldDetailForm.php b/forms/gridfield/GridFieldDetailForm.php index 70fb7d8c7..919b8bb9f 100644 --- a/forms/gridfield/GridFieldDetailForm.php +++ b/forms/gridfield/GridFieldDetailForm.php @@ -1,4 +1,5 @@ name = $name; + $this->constructExtensions(); } /** @@ -88,10 +92,7 @@ class GridFieldDetailForm implements GridField_URLHandler { $record = Object::create($gridField->getModelClass()); } - $class = $this->getItemRequestClass(); - - $handler = Object::create($class, $gridField, $this, $record, $requestHandler, $this->name); - $handler->setTemplate($this->template); + $handler = $this->getItemRequestHandler($gridField, $record, $requestHandler); // if no validator has been set on the GridField and the record has a // CMS validator, use that. @@ -102,6 +103,26 @@ class GridFieldDetailForm implements GridField_URLHandler { return $handler->handleRequest($request, DataModel::inst()); } + /** + * Build a request handler for the given record + * + * @param GridField $gridField + * @param DataObject $record + * @param Controller $requestHandler + * @return GridFieldDetailForm_ItemRequest + */ + protected function getItemRequestHandler($gridField, $record, $requestHandler) { + $class = $this->getItemRequestClass(); + $this->extend('updateItemRequestClass', $class, $gridField, $record, $requestHandler); + $handler = \Injector::inst()->createWithArgs( + $class, + array($gridField, $this, $record, $requestHandler, $this->name) + ); + $handler->setTemplate($this->template); + $this->extend('updateItemRequestHandler', $handler); + return $handler; + } + /** * @param String */ @@ -351,41 +372,8 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { return $controller->httpError(403); } - $actions = new FieldList(); - if($this->record->ID !== 0) { - if($canEdit) { - $actions->push(FormAction::create('doSave', _t('GridFieldDetailForm.Save', 'Save')) - ->setUseButtonTag(true) - ->addExtraClass('ss-ui-action-constructive') - ->setAttribute('data-icon', 'accept')); - } - - if($canDelete) { - $actions->push(FormAction::create('doDelete', _t('GridFieldDetailForm.Delete', 'Delete')) - ->setUseButtonTag(true) - ->addExtraClass('ss-ui-action-destructive action-delete')); - } - - }else{ // adding new record - //Change the Save label to 'Create' - $actions->push(FormAction::create('doSave', _t('GridFieldDetailForm.Create', 'Create')) - ->setUseButtonTag(true) - ->addExtraClass('ss-ui-action-constructive') - ->setAttribute('data-icon', 'add')); - - // Add a Cancel link which is a button-like link and link back to one level up. - $curmbs = $this->Breadcrumbs(); - if($curmbs && $curmbs->count()>=2){ - $one_level_up = $curmbs->offsetGet($curmbs->count()-2); - $text = sprintf( - "%s", - "crumb ss-ui-button ss-ui-action-destructive cms-panel-link ui-corner-all", // CSS classes - $one_level_up->Link, // url - _t('GridFieldDetailForm.CancelBtn', 'Cancel') // label - ); - $actions->push(new LiteralField('cancelbutton', $text)); - } - } + // Build actions + $actions = $this->getFormActions(); $fields = $this->component->getFields(); if(!$fields) $fields = $this->record->getCMSFields(); @@ -462,6 +450,53 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { return $form; } + /** + * Build the set of form field actions for this DataObject + * + * @return FieldList + */ + protected function getFormActions() { + $canEdit = $this->record->canEdit(); + $canDelete = $this->record->canDelete(); + $actions = new FieldList(); + if($this->record->ID !== 0) { + if($canEdit) { + $actions->push(FormAction::create('doSave', _t('GridFieldDetailForm.Save', 'Save')) + ->setUseButtonTag(true) + ->addExtraClass('ss-ui-action-constructive') + ->setAttribute('data-icon', 'accept')); + } + + if($canDelete) { + $actions->push(FormAction::create('doDelete', _t('GridFieldDetailForm.Delete', 'Delete')) + ->setUseButtonTag(true) + ->addExtraClass('ss-ui-action-destructive action-delete')); + } + + } else { // adding new record + //Change the Save label to 'Create' + $actions->push(FormAction::create('doSave', _t('GridFieldDetailForm.Create', 'Create')) + ->setUseButtonTag(true) + ->addExtraClass('ss-ui-action-constructive') + ->setAttribute('data-icon', 'add')); + + // Add a Cancel link which is a button-like link and link back to one level up. + $crumbs = $this->Breadcrumbs(); + if($crumbs && $crumbs->count() >= 2){ + $oneLevelUp = $crumbs->offsetGet($crumbs->count() - 2); + $text = sprintf( + "%s", + "crumb ss-ui-button ss-ui-action-destructive cms-panel-link ui-corner-all", // CSS classes + $oneLevelUp->Link, // url + _t('GridFieldDetailForm.CancelBtn', 'Cancel') // label + ); + $actions->push(new LiteralField('cancelbutton', $text)); + } + } + $this->extend('updateFormActions', $actions); + return $actions; + } + /** * Traverse the nested RequestHandlers until we reach something that's not GridFieldDetailForm_ItemRequest. * This allows us to access the Controller responsible for invoking the top-level GridField. @@ -525,47 +560,20 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { } public function doSave($data, $form) { - $new_record = $this->record->ID == 0; - $controller = $this->getToplevelController(); - $list = $this->gridField->getList(); + $isNewRecord = $this->record->ID == 0; - if(!$this->record->canEdit()) { - return $controller->httpError(403); - } - - if (isset($data['ClassName']) && $data['ClassName'] != $this->record->ClassName) { - $newClassName = $data['ClassName']; - // The records originally saved attribute was overwritten by $form->saveInto($record) before. - // This is necessary for newClassInstance() to work as expected, and trigger change detection - // on the ClassName attribute - $this->record->setClassName($this->record->ClassName); - // Replace $record with a new instance - $this->record = $this->record->newClassInstance($newClassName); + // Check permission + if (!$this->record->canEdit()) { + return $this->httpError(403); } + // Save from form data try { - $form->saveInto($this->record); - $this->record->write(); - $extraData = $this->getExtraSavedData($this->record, $list); - $list->add($this->record, $extraData); - } catch(ValidationException $e) { - $form->sessionMessage($e->getResult()->message(), 'bad', false); - $responseNegotiator = new PjaxResponseNegotiator(array( - 'CurrentForm' => function() use(&$form) { - return $form->forTemplate(); - }, - 'default' => function() use(&$controller) { - return $controller->redirectBack(); - } - )); - if($controller->getRequest()->isAjax()){ - $controller->getRequest()->addHeader('X-Pjax', 'CurrentForm'); - } - return $responseNegotiator->respond($controller->getRequest()); + $this->saveFormIntoRecord($data, $form); + } catch (ValidationException $e) { + return $this->generateValidationResponse($form, $e); } - // TODO Save this item into the given relationship - $link = '"' . htmlspecialchars($this->record->Title, ENT_QUOTES) . '"'; @@ -580,7 +588,19 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { $form->sessionMessage($message, 'good', false); - if($new_record) { + // Redirect after save + return $this->redirectAfterSave($isNewRecord); + } + + /** + * Response object for this request after a successful save + * + * @param bool $isNewRecord True if this record was just created + * @return SS_HTTPResponse|HTMLText + */ + protected function redirectAfterSave($isNewRecord) { + $controller = $this->getToplevelController(); + if($isNewRecord) { return $controller->redirect($this->Link()); } elseif($this->gridField->getList()->byId($this->record->ID)) { // Return new view, as we can't do a "virtual redirect" via the CMS Ajax @@ -596,6 +616,69 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { } } + public function httpError($errorCode, $errorMessage = null) { + $controller = $this->getToplevelController(); + return $controller->httpError($errorCode, $errorMessage); + } + + /** + * Loads the given form data into the underlying dataobject and relation + * + * @param array $data + * @param Form $form + * @throws ValidationException On error + * @return DataObject Saved record + */ + protected function saveFormIntoRecord($data, $form) { + $list = $this->gridField->getList(); + + // Check object matches the correct classname + if (isset($data['ClassName']) && $data['ClassName'] != $this->record->ClassName) { + $newClassName = $data['ClassName']; + // The records originally saved attribute was overwritten by $form->saveInto($record) before. + // This is necessary for newClassInstance() to work as expected, and trigger change detection + // on the ClassName attribute + $this->record->setClassName($this->record->ClassName); + // Replace $record with a new instance + $this->record = $this->record->newClassInstance($newClassName); + } + + // Save form and any extra saved data into this dataobject + $form->saveInto($this->record); + $this->record->write(); + $extraData = $this->getExtraSavedData($this->record, $list); + $list->add($this->record, $extraData); + + return $this->record; + } + + /** + * Generate a response object for a form validation error + * + * @param Form $form The source form + * @param ValidationException $e The validation error message + * @return SS_HTTPResponse + * @throws SS_HTTPResponse_Exception + */ + protected function generateValidationResponse($form, $e) { + $controller = $this->getToplevelController(); + + $form->sessionMessage($e->getResult()->message(), 'bad', false); + $responseNegotiator = new PjaxResponseNegotiator(array( + 'CurrentForm' => function() use(&$form) { + return $form->forTemplate(); + }, + 'default' => function() use(&$controller) { + return $controller->redirectBack(); + } + )); + if($controller->getRequest()->isAjax()){ + $controller->getRequest()->addHeader('X-Pjax', 'CurrentForm'); + } + return $responseNegotiator->respond($controller->getRequest()); + } + + public function doDelete($data, $form) { $title = $this->record->Title; try { diff --git a/model/DataExtension.php b/model/DataExtension.php index d09a99e06..3579ccc4d 100644 --- a/model/DataExtension.php +++ b/model/DataExtension.php @@ -2,6 +2,8 @@ /** * An extension that adds additional functionality to a {@link DataObject}. * + * @property DataObject $owner + * * @package framework * @subpackage model */ diff --git a/model/Versioned.php b/model/versioning/Versioned.php similarity index 87% rename from model/Versioned.php rename to model/versioning/Versioned.php index c3a0b9b1a..a565ddcf2 100644 --- a/model/Versioned.php +++ b/model/versioning/Versioned.php @@ -1,5 +1,7 @@ migrateVersion(null); } + /** + * This function should return true if the current user can publish this record. + * It can be overloaded to customise the security model for an application. + * + * Denies permission if any of the following conditions is true: + * - canPublish() on any extension returns false + * - canEdit() returns false + * + * @param Member $member + * @return bool True if the current user can publish this record. + */ + public function canPublish($member = null) { + // Skip if invoked by extendedCan() + if(func_num_args() > 4) { + return null; + } + + if(!$member) { + $member = Member::currentUser(); + } + + if(Permission::checkMember($member, "ADMIN")) { + return true; + } + + // Standard mechanism for accepting permission changes from extensions + $extended = $this->owner->extendedCan('canPublish', $member); + if($extended !== null) { + return $extended; + } + + // Default to relying on edit permission + return $this->owner->canEdit($member); + } + + /** + * Check if the current user can delete this record from live + * + * @param null $member + * @return mixed + */ + public function canUnpublish($member = null) { + // Skip if invoked by extendedCan() + if(func_num_args() > 4) { + return null; + } + + if(!$member) { + $member = Member::currentUser(); + } + + if(Permission::checkMember($member, "ADMIN")) { + return true; + } + + // Standard mechanism for accepting permission changes from extensions + $extended = $this->owner->extendedCan('canUnpublish', $member); + if($extended !== null) { + return $extended; + } + + // Default to relying on canPublish + return $this->owner->canPublish($member); + } + + /** + * Check if the current user is allowed to archive this record. + * If extended, ensure that both canDelete and canUnpublish are extended also + * + * @param Member $member + * @return bool + */ + public function canArchive($member = null) { + // Skip if invoked by extendedCan() + if(func_num_args() > 4) { + return null; + } + + if(!$member) { + $member = Member::currentUser(); + } + + if(Permission::checkMember($member, "ADMIN")) { + return true; + } + + // Standard mechanism for accepting permission changes from extensions + $extended = $this->owner->extendedCan('canArchive', $member); + if($extended !== null) { + return $extended; + } + + // Check if this record can be deleted from stage + if(!$this->owner->canDelete($member)) { + return false; + } + + // Check if we can delete from live + if(!$this->owner->canUnpublish($member)) { + return false; + } + + return true; + } + /** * Extend permissions to include additional security for objects that are not published to live. * @@ -788,6 +893,11 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { return true; } + // Bypass if record doesn't have a live stage + if(!in_array(static::get_live_stage(), $this->getVersionedStages())) { + return true; + } + // If we weren't definitely loaded from live, and we can't view non-live content, we need to // check to make sure this version is the live version and so can be viewed $latestVersion = Versioned::get_versionnumber_by_stage($this->owner->class, 'Live', $this->owner->ID); @@ -900,6 +1010,77 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { )->value(); } + /** + * Provides a simple doPublish action for Versioned dataobjects + * + * @return bool True if publish was successful + */ + public function doPublish() { + $owner = $this->owner; + $owner->invokeWithExtensions('onBeforePublish'); + $owner->write(); + $owner->publish("Stage", "Live"); + $owner->invokeWithExtensions('onAfterPublish'); + return true; + } + + + + /** + * Removes the record from both live and stage + * + * @return bool Success + */ + public function doArchive() { + $owner = $this->owner; + $owner->invokeWithExtensions('onBeforeArchive', $this); + + if($owner->doUnpublish()) { + $owner->delete(); + $owner->invokeWithExtensions('onAfterArchive', $this); + + return true; + } + + return false; + } + + /** + * Removes this record from the live site + * + * @return bool Flag whether the unpublish was successful + * + * @uses SiteTreeExtension->onBeforeUnpublish() + * @uses SiteTreeExtension->onAfterUnpublish() + */ + public function doUnpublish() { + $owner = $this->owner; + if(!$owner->isInDB()) { + return false; + } + + $owner->invokeWithExtensions('onBeforeUnpublish'); + + $origStage = self::current_stage(); + self::reading_stage(self::get_live_stage()); + + // This way our ID won't be unset + $clone = clone $owner; + $clone->delete(); + + self::reading_stage($origStage); + + // If we're on the draft site, then we can update the status. + // Otherwise, these lines will resurrect an inappropriate record + if(self::current_stage() != self::get_live_stage() && $this->isOnDraft()) { + $owner->write(); + } + + $owner->invokeWithExtensions('onAfterUnpublish'); + + return true; + } + /** * Move a database record from one stage to the other. * @@ -909,50 +1090,55 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * By default, the existing version number will be copied over. */ public function publish($fromStage, $toStage, $createNewVersion = false) { - $this->owner->extend('onBeforeVersionedPublish', $fromStage, $toStage, $createNewVersion); + $owner = $this->owner; + $owner->extend('onBeforeVersionedPublish', $fromStage, $toStage, $createNewVersion); - $baseClass = $this->owner->class; + $baseClass = $owner->class; while( ($p = get_parent_class($baseClass)) != "DataObject") $baseClass = $p; $extTable = $this->extendWithSuffix($baseClass); if(is_numeric($fromStage)) { - $from = Versioned::get_version($baseClass, $this->owner->ID, $fromStage); + $from = Versioned::get_version($baseClass, $owner->ID, $fromStage); } else { - $this->owner->flushCache(); - $from = Versioned::get_one_by_stage($baseClass, $fromStage, "\"{$baseClass}\".\"ID\"={$this->owner->ID}"); + $owner->flushCache(); + $from = Versioned::get_one_by_stage($baseClass, $fromStage, "\"{$baseClass}\".\"ID\"={$owner->ID}"); + } + if(!$from) { + throw new InvalidArgumentException("Can't find {$baseClass}#{$owner->ID} in stage {$fromStage}"); } $publisherID = isset(Member::currentUser()->ID) ? Member::currentUser()->ID : 0; - if($from) { - $from->forceChange(); - if($createNewVersion) { - $latest = self::get_latest_version($baseClass, $this->owner->ID); - $this->owner->Version = $latest->Version + 1; - } else { - $from->migrateVersion($from->Version); - } - - // Mark this version as having been published at some stage - DB::prepared_query("UPDATE \"{$extTable}_versions\" - SET \"WasPublished\" = ?, \"PublisherID\" = ? - WHERE \"RecordID\" = ? AND \"Version\" = ?", - array(1, $publisherID, $from->ID, $from->Version) - ); - - $oldMode = Versioned::get_reading_mode(); - Versioned::reading_stage($toStage); - - $conn = DB::get_conn(); - if(method_exists($conn, 'allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing($baseClass, true); - $from->write(); - if(method_exists($conn, 'allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing($baseClass, false); - - $from->destroy(); - - Versioned::set_reading_mode($oldMode); + $from->forceChange(); + if($createNewVersion) { + $latest = self::get_latest_version($baseClass, $owner->ID); + $owner->Version = $latest->Version + 1; } else { - user_error("Can't find {$this->owner->URLSegment}/{$this->owner->ID} in stage $fromStage", E_USER_WARNING); + $from->migrateVersion($from->Version); } + + // Mark this version as having been published at some stage + DB::prepared_query("UPDATE \"{$extTable}_versions\" + SET \"WasPublished\" = ?, \"PublisherID\" = ? + WHERE \"RecordID\" = ? AND \"Version\" = ?", + array(1, $publisherID, $from->ID, $from->Version) + ); + + $oldMode = Versioned::get_reading_mode(); + Versioned::reading_stage($toStage); + + $conn = DB::get_conn(); + if(method_exists($conn, 'allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing($baseClass, true); + // Migrate stage prior to write + $from->setSourceQueryParam('Versioned.mode', 'stage'); + $from->setSourceQueryParam('Versioned.stage', $toStage); + $from->write(); + if(method_exists($conn, 'allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing($baseClass, false); + + $from->destroy(); + + Versioned::set_reading_mode($oldMode); + + $owner->extend('onAfterVersionedPublish', $fromStage, $toStage, $createNewVersion); } /** @@ -1274,7 +1460,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { return self::$cache_versionnumber[$baseClass][$stage][$id]; } - // get version as performance-optimized SQL query (gets called for each page in the sitetree) + // get version as performance-optimized SQL query (gets called for each record in the sitetree) $version = DB::prepared_query( "SELECT \"Version\" FROM \"$stageTable\" WHERE \"ID\" = ?", array($id) @@ -1299,7 +1485,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { /** * Pre-populate the cache for Versioned::get_versionnumber_by_stage() for * a list of record IDs, for more efficient database querying. If $idList - * is null, then every page will be pre-cached. + * is null, then every record will be pre-cached. * * @param string $class * @param string $stage @@ -1391,22 +1577,21 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { } /** - * Roll the draft version of this page to match the published page. + * Roll the draft version of this record to match the published record. * Caution: Doesn't overwrite the object properties with the rolled back version. * * @param int $version Either the string 'Live' or a version number */ public function doRollbackTo($version) { - $this->owner->extend('onBeforeRollback', $version); + $owner = $this->owner; + $owner->extend('onBeforeRollback', $version); $this->publish($version, "Stage", true); - - $this->owner->writeWithoutVersion(); - - $this->owner->extend('onAfterRollback', $version); + $owner->writeWithoutVersion(); + $owner->extend('onAfterRollback', $version); } /** - * Return the latest version of the given page. + * Return the latest version of the given record. * * @return DataObject */ @@ -1430,14 +1615,55 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return boolean */ public function isLatestVersion() { - $version = self::get_latest_version($this->owner->class, $this->owner->ID); + if(!$this->owner->isInDB()) { + return false; + } + $version = self::get_latest_version($this->owner->class, $this->owner->ID); return ($version->Version == $this->owner->Version); } + /** + * Check if this record exists on live + * + * @return bool + */ + public function isPublished() { + if(!$this->owner->isInDB()) { + return false; + } + + $table = ClassInfo::baseDataClass($this->owner->class) . '_' . self::get_live_stage(); + $result = DB::prepared_query( + "SELECT COUNT(*) FROM \"{$table}\" WHERE \"{$table}\".\"ID\" = ?", + array($this->owner->ID) + ); + return (bool)$result->value(); + } + + /** + * Check if this record exists on the draft stage + * + * @return bool + */ + public function isOnDraft() { + if(!$this->owner->isInDB()) { + return false; + } + + $table = ClassInfo::baseDataClass($this->owner->class); + $result = DB::prepared_query( + "SELECT COUNT(*) FROM \"{$table}\" WHERE \"{$table}\".\"ID\" = ?", + array($this->owner->ID) + ); + return (bool)$result->value(); + } + + + /** * Return the equivalent of a DataList::create() call, querying the latest - * version of each page stored in the (class)_versions tables. + * version of each record stored in the (class)_versions tables. * * In particular, this will query deleted records as well as active ones. * @@ -1498,7 +1724,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @param array $labels */ public function updateFieldLabels(&$labels) { - $labels['Versions'] = _t('Versioned.has_many_Versions', 'Versions', 'Past Versions of this page'); + $labels['Versions'] = _t('Versioned.has_many_Versions', 'Versions', 'Past Versions of this record'); } /** diff --git a/model/versioning/VersionedGridFieldDetailForm.php b/model/versioning/VersionedGridFieldDetailForm.php new file mode 100644 index 000000000..fd746eb8e --- /dev/null +++ b/model/versioning/VersionedGridFieldDetailForm.php @@ -0,0 +1,13 @@ +has_extension('Versioned')) { + $class = 'VersionedGridFieldItemRequest'; + } + } +} diff --git a/model/versioning/VersionedGridFieldItemRequest.php b/model/versioning/VersionedGridFieldItemRequest.php new file mode 100644 index 000000000..782fb1d73 --- /dev/null +++ b/model/versioning/VersionedGridFieldItemRequest.php @@ -0,0 +1,196 @@ +getRecord(); + if(!$record || !$record->has_extension('Versioned')) { + return $actions; + } + + // Save & Publish action + if($record->canPublish()) { + // "publish", as with "save", it supports an alternate state to show when action is needed. + $publish = FormAction::create( + 'doPublish', + _t('VersionedGridFieldItemRequest.BUTTONPUBLISH', 'Publish') + ) + ->setUseButtonTag(true) + ->addExtraClass('ss-ui-action-constructive') + ->setAttribute('data-icon', 'accept'); + + // Insert after save + if($actions->fieldByName('action_doSave')) { + $actions->insertAfter('action_doSave', $publish); + } else { + $actions->push($publish); + } + } + + // Unpublish action + $isPublished = $record->isPublished(); + if($isPublished && $record->canUnpublish()) { + $actions->push( + FormAction::create( + 'doUnpublish', + _t('VersionedGridFieldItemRequest.BUTTONUNPUBLISH', 'Unpublish') + ) + ->setUseButtonTag(true) + ->setDescription(_t( + 'VersionedGridFieldItemRequest.BUTTONUNPUBLISHDESC', + 'Remove this record from the published site' + )) + ->addExtraClass('ss-ui-action-destructive') + ); + } + + // Archive action + if($record->canArchive()) { + // Replace "delete" action + $actions->removeByName('action_doDelete'); + + // "archive" + $actions->push( + FormAction::create('doArchive', _t('VersionedGridFieldItemRequest.ARCHIVE','Archive')) + ->setDescription(_t( + 'VersionedGridFieldItemRequest.BUTTONARCHIVEDESC', + 'Unpublish and send to archive' + )) + ->addExtraClass('delete ss-ui-action-destructive') + ); + } + return $actions; + } + + /** + * Archive this versioned record + * + * @param array $data + * @param Form $form + * @return SS_HTTPResponse + */ + public function doArchive($data, $form) { + $record = $this->getRecord(); + if (!$record->canArchive()) { + return $this->httpError(403); + } + + // Record name before it's deleted + $title = $record->Title; + + try { + $record->doArchive(); + } catch(ValidationException $e) { + return $this->generateValidationResponse($form, $e); + } + + $message = sprintf( + _t('VersionedGridFieldItemRequest.Archived', 'Archived %s %s'), + $record->i18n_singular_name(), + Convert::raw2xml($title) + ); + $this->setFormMessage($form, $message); + + //when an item is deleted, redirect to the parent controller + $controller = $this->getToplevelController(); + $controller->getRequest()->addHeader('X-Pjax', 'Content'); // Force a content refresh + + return $controller->redirect($this->getBacklink(), 302); //redirect back to admin section + } + + /** + * Publish this versioned record + * + * @param array $data + * @param Form $form + * @return SS_HTTPResponse + */ + public function doPublish($data, $form) { + $record = $this->getRecord(); + $isNewRecord = $record->ID == 0; + + // Check permission + if(!$record->canPublish()) { + return $this->httpError(403); + } + + // Save from form data + try { + // Initial save and reload + $record = $this->saveFormIntoRecord($data, $form); + $record->doPublish(); + + } catch(ValidationException $e) { + return $this->generateValidationResponse($form, $e); + } + + $editURL = $this->Link('edit'); + $xmlTitle = Convert::raw2xml($record->Title); + $link = "{$xmlTitle}"; + $message = _t( + 'VersionedGridFieldItemRequest.Published', + 'Published {name} {link}', + array( + 'name' => $record->i18n_singular_name(), + 'link' => $link + ) + ); + $this->setFormMessage($form, $message); + + return $this->redirectAfterSave($isNewRecord); + } + + /** + * Delete this record from the live site + * + * @param array $data + * @param Form $form + * @return SS_HTTPResponse + */ + public function doUnpublish($data, $form) { + $record = $this->getRecord(); + if (!$record->canUnpublish()) { + return $this->httpError(403); + } + + // Record name before it's deleted + $title = $record->Title; + + try { + $record->doUnpublish(); + } catch(ValidationException $e) { + return $this->generateValidationResponse($form, $e); + } + + $message = sprintf( + _t('VersionedGridFieldItemRequest.Unpublished', 'Unpublished %s %s'), + $record->i18n_singular_name(), + Convert::raw2xml($title) + ); + $this->setFormMessage($form, $message); + + // Redirect back to edit + return $this->redirectAfterSave(false); + } + + /** + * @param Form $form + * @param string $message + */ + protected function setFormMessage($form, $message) { + $form->sessionMessage($message, 'good', false); + $controller = $this->getToplevelController(); + if($controller->hasMethod('getEditForm')) { + $backForm = $controller->getEditForm(); + $backForm->sessionMessage($message, 'good', false); + } + } +} diff --git a/parsers/ShortcodeHandler.php b/parsers/ShortcodeHandler.php index d3a89d76c..03f98a104 100644 --- a/parsers/ShortcodeHandler.php +++ b/parsers/ShortcodeHandler.php @@ -5,6 +5,13 @@ */ interface ShortcodeHandler { + /** + * Gets the list of shortcodes provided by this handler + * + * @return mixed + */ + public static function get_shortcodes(); + /** * Generate content with a shortcode value * diff --git a/tasks/MigrateFileTask.php b/tasks/MigrateFileTask.php index 8894d6921..b9f476c31 100644 --- a/tasks/MigrateFileTask.php +++ b/tasks/MigrateFileTask.php @@ -17,6 +17,8 @@ class MigrateFileTask extends BuildTask { $migrated = FileMigrationHelper::singleton()->run(); if($migrated) { DB::alteration_message("{$migrated} File DataObjects upgraded", "changed"); + } else { + DB::alteration_message("No File DataObjects need upgrading", "notice"); } } diff --git a/tests/filesystem/AssetControlExtensionTest.php b/tests/filesystem/AssetControlExtensionTest.php index 0744d54e9..7027569ec 100644 --- a/tests/filesystem/AssetControlExtensionTest.php +++ b/tests/filesystem/AssetControlExtensionTest.php @@ -15,7 +15,9 @@ class AssetControlExtensionTest extends SapphireTest { parent::setUp(); // Set backend and base url + \Versioned::reading_stage('Stage'); AssetStoreTest_SpyStore::activate('AssetControlExtensionTest'); + $this->logInWithPermission('ADMIN'); // Setup fixture manually $object1 = new AssetControlExtensionTest_VersionedObject(); @@ -24,7 +26,7 @@ class AssetControlExtensionTest extends SapphireTest { $object1->Header->setFromLocalFile($fish1, 'Header/MyObjectHeader.jpg'); $object1->Download->setFromString('file content', 'Documents/File.txt'); $object1->write(); - $object1->publish('Stage', 'Live'); + $object1->doPublish(); $object2 = new AssetControlExtensionTest_Object(); $object2->Title = 'Unversioned'; @@ -35,7 +37,7 @@ class AssetControlExtensionTest extends SapphireTest { $object3->Title = 'Archived'; $object3->Header->setFromLocalFile($fish1, 'Archived/MyObjectHeader.jpg'); $object3->write(); - $object3->publish('Stage', 'Live'); + $object3->doPublish(); } public function tearDown() { @@ -44,6 +46,8 @@ class AssetControlExtensionTest extends SapphireTest { } public function testFileDelete() { + \Versioned::reading_stage('Stage'); + /** @var AssetControlExtensionTest_VersionedObject $object1 */ $object1 = AssetControlExtensionTest_VersionedObject::get() ->filter('Title', 'My object') @@ -111,6 +115,87 @@ class AssetControlExtensionTest extends SapphireTest { $this->assertNull($object2->Image->getVisibility()); $this->assertEquals(AssetStore::VISIBILITY_PROTECTED, $object3->Header->getVisibility()); } + + /** + * Test files being replaced + */ + public function testReplaceFile() { + \Versioned::reading_stage('Stage'); + + /** @var AssetControlExtensionTest_VersionedObject $object1 */ + $object1 = AssetControlExtensionTest_VersionedObject::get() + ->filter('Title', 'My object') + ->first(); + /** @var AssetControlExtensionTest_Object $object2 */ + $object2 = AssetControlExtensionTest_Object::get() + ->filter('Title', 'Unversioned') + ->first(); + + /** @var AssetControlExtensionTest_ArchivedObject $object3 */ + $object3 = AssetControlExtensionTest_ArchivedObject::get() + ->filter('Title', 'Archived') + ->first(); + + $object1TupleOld = $object1->Header->getValue(); + $object2TupleOld = $object2->Image->getValue(); + $object3TupleOld = $object3->Header->getValue(); + + // Replace image and write each to filesystem + $fish1 = realpath(__DIR__ .'/../model/testimages/test-image-high-quality.jpg'); + $object1->Header->setFromLocalFile($fish1, 'Header/Replaced_MyObjectHeader.jpg'); + $object1->write(); + $object2->Image->setFromLocalFile($fish1, 'Images/Replaced_BeautifulFish.jpg'); + $object2->write(); + $object3->Header->setFromLocalFile($fish1, 'Archived/Replaced_MyObjectHeader.jpg'); + $object3->write(); + + // Check that old published records are left public, but removed for unversioned object2 + $this->assertEquals( + AssetStore::VISIBILITY_PUBLIC, + $this->getAssetStore()->getVisibility($object1TupleOld['Filename'], $object1TupleOld['Hash']) + ); + $this->assertEquals( + null, // Old file is destroyed + $this->getAssetStore()->getVisibility($object2TupleOld['Filename'], $object2TupleOld['Hash']) + ); + $this->assertEquals( + AssetStore::VISIBILITY_PUBLIC, + $this->getAssetStore()->getVisibility($object3TupleOld['Filename'], $object3TupleOld['Hash']) + ); + + // Check that visibility of new file is correct + // Note that $object2 has no canView() is true, so assets end up public + $this->assertEquals(AssetStore::VISIBILITY_PROTECTED, $object1->Header->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object2->Image->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PROTECTED, $object3->Header->getVisibility()); + + // Publish changes to versioned records + $object1->doPublish(); + $object3->doPublish(); + + // After publishing, old object1 is deleted, but since object3 has archiving enabled, + // the orphaned file is intentionally left in the protected store + $this->assertEquals( + null, + $this->getAssetStore()->getVisibility($object1TupleOld['Filename'], $object1TupleOld['Hash']) + ); + $this->assertEquals( + AssetStore::VISIBILITY_PROTECTED, + $this->getAssetStore()->getVisibility($object3TupleOld['Filename'], $object3TupleOld['Hash']) + ); + + // And after publish, all files are public + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object1->Header->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object3->Header->getVisibility()); + } + + /** + * @return AssetStore + */ + protected function getAssetStore() { + return Injector::inst()->get('AssetStore'); + } + } /** @@ -131,6 +216,25 @@ class AssetControlExtensionTest_VersionedObject extends DataObject implements Te 'Header' => "DBFile('image/supported')", 'Download' => 'DBFile' ); + + /** + * @param Member $member + * @return bool + */ + public function canView($member = null) { + if(!$member) { + $member = Member::currentUser(); + } + + // Expectation that versioned::canView will hide this object in draft + $result = $this->extendedCan('canView', $member); + if($result !== null) { + return $result; + } + + // Open to public + return true; + } } /** @@ -144,11 +248,20 @@ class AssetControlExtensionTest_Object extends DataObject implements TestOnly { 'Title' => 'Varchar(255)', 'Image' => "DBFile('image/supported')" ); + + + /** + * @param Member $member + * @return bool + */ + public function canView($member = null) { + return true; + } } /** * Versioned object that always archives its assets */ class AssetControlExtensionTest_ArchivedObject extends AssetControlExtensionTest_VersionedObject { - private static $archive_assets = true; + private static $keep_archived_assets = true; } diff --git a/tests/filesystem/AssetManipulationListTest.php b/tests/filesystem/AssetManipulationListTest.php new file mode 100644 index 000000000..eef0e544a --- /dev/null +++ b/tests/filesystem/AssetManipulationListTest.php @@ -0,0 +1,79 @@ + 'Test1.jpg', 'Hash' => '975677589962604d9e16b700cf84734f9dda2817']; + $file2 = ['Filename' => 'Test2.jpg', 'Hash' => '22af86a45ea56287437a12cf83aded5c077a5db5']; + $file3 = ['Filename' => 'DupeHash1.jpg', 'Hash' => 'f167433dd318e738281b845a07d7be2053b8c997']; + $file4 = ['Filename' => 'DupeName.jpg', 'Hash' => 'afde6577a034323959b7915f41ac8d1f53bc597f']; + $file5 = ['Filename' => 'DupeName.jpg', 'Hash' => '1e94b066e5aa16907d0e5e32556c7a2a0b692eb9']; + $file6 = ['Filename' => 'DupeHash2.jpg', 'Hash' => 'f167433dd318e738281b845a07d7be2053b8c997']; + + // Non-overlapping assets remain in assigned sets + $this->assertTrue($set->addDeletedAsset($file1)); + $this->assertTrue($set->addDeletedAsset($file2)); + $this->assertTrue($set->addProtectedAsset($file3)); + $this->assertTrue($set->addProtectedAsset($file4)); + $this->assertTrue($set->addPublicAsset($file5)); + $this->assertTrue($set->addPublicAsset($file6)); + + // Check initial state of list + $this->assertEquals(6, $this->countItems($set)); + $this->assertContains($file1, $set->getDeletedAssets()); + $this->assertContains($file2, $set->getDeletedAssets()); + $this->assertContains($file3, $set->getProtectedAssets()); + $this->assertContains($file4, $set->getProtectedAssets()); + $this->assertContains($file5, $set->getPublicAssets()); + $this->assertContains($file6, $set->getPublicAssets()); + + // Public or Protected assets will not be deleted + $this->assertFalse($set->addDeletedAsset($file3)); + $this->assertFalse($set->addDeletedAsset($file4)); + $this->assertFalse($set->addDeletedAsset($file5)); + $this->assertFalse($set->addDeletedAsset($file6)); + $this->assertEquals(6, $this->countItems($set)); + $this->assertNotContains($file3, $set->getDeletedAssets()); + $this->assertNotContains($file4, $set->getDeletedAssets()); + $this->assertNotContains($file5, $set->getDeletedAssets()); + $this->assertNotContains($file6, $set->getDeletedAssets()); + + // Adding records as protected will remove them from the deletion list, but + // not the public list + $this->assertTrue($set->addProtectedAsset($file1)); + $this->assertFalse($set->addProtectedAsset($file5)); + $this->assertEquals(6, $this->countItems($set)); + $this->assertNotContains($file1, $set->getDeletedAssets()); + $this->assertContains($file1, $set->getProtectedAssets()); + $this->assertNotContains($file5, $set->getProtectedAssets()); + $this->assertContains($file5, $set->getPublicAssets()); + + // Adding records as public will ensure they are not deleted or marked as protected + // Existing public assets won't be re-added + $this->assertTrue($set->addPublicAsset($file2)); + $this->assertTrue($set->addPublicAsset($file4)); + $this->assertFalse($set->addPublicAsset($file5)); + $this->assertEquals(6, $this->countItems($set)); + $this->assertNotContains($file2, $set->getDeletedAssets()); + $this->assertNotContains($file2, $set->getProtectedAssets()); + $this->assertContains($file2, $set->getPublicAssets()); + $this->assertNotContains($file4, $set->getProtectedAssets()); + $this->assertContains($file4, $set->getPublicAssets()); + $this->assertContains($file5, $set->getPublicAssets()); + } + + /** + * Helper to count all items in a set + * + * @param AssetManipulationList $set + * @return int + */ + protected function countItems(AssetManipulationList $set) { + return count($set->getPublicAssets()) + count($set->getProtectedAssets()) + count($set->getDeletedAssets()); + } +} diff --git a/tests/filesystem/FileMigrationHelperTest.php b/tests/filesystem/FileMigrationHelperTest.php index 6fa3aaa1e..a43871118 100644 --- a/tests/filesystem/FileMigrationHelperTest.php +++ b/tests/filesystem/FileMigrationHelperTest.php @@ -60,6 +60,7 @@ class FileMigrationHelperTest extends SapphireTest { $this->assertEmpty($file->File->getFilename(), "File {$file->Name} has no DBFile filename"); $this->assertEmpty($file->File->getHash(), "File {$file->Name} has no hash"); $this->assertFalse($file->exists(), "File with name {$file->Name} does not yet exist"); + $this->assertFalse($file->isPublished(), "File is not published yet"); } // Do migration @@ -77,6 +78,7 @@ class FileMigrationHelperTest extends SapphireTest { "File with name {$filename} has the correct hash" ); $this->assertTrue($file->exists(), "File with name {$filename} exists"); + $this->assertTrue($file->isPublished(), "File is published after migration"); } } diff --git a/tests/filesystem/FileTest.php b/tests/filesystem/FileTest.php index 7301758ac..95d138a38 100644 --- a/tests/filesystem/FileTest.php +++ b/tests/filesystem/FileTest.php @@ -1,6 +1,7 @@ logInWithPermission('ADMIN'); + Versioned::reading_stage('Stage'); // Set backend root to /ImageTest AssetStoreTest_SpyStore::activate('FileTest'); @@ -232,40 +235,92 @@ class FileTest extends SapphireTest { public function testSetNameChangesFilesystemOnWrite() { $file = $this->objFromFixture('File', 'asdf'); - $oldPath = AssetStoreTest_SpyStore::getLocalPath($file); - $newPath = str_replace('FileTest.txt', 'renamed.txt', $oldPath); + $this->logInWithPermission('ADMIN'); + $file->doPublish(); + $oldTuple = $file->File->getValue(); + + // Rename + $file->Name = 'renamed.txt'; + $newTuple = $oldTuple; + $newTuple['Filename'] = $file->getFilename(); // Before write() - $file->Name = 'renamed.txt'; - $this->assertFileExists($oldPath, 'Old path is still present'); - $this->assertFileNotExists($newPath, 'New path is updated in memory, not written before write() is called'); - $file->write(); + $this->assertTrue( + $this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']), + 'Old path is still present' + ); + $this->assertFalse( + $this->getAssetStore()->exists($newTuple['Filename'], $newTuple['Hash']), + 'New path is updated in memory, not written before write() is called' + ); // After write() - $this->assertFileExists($oldPath, 'Old path is left after write()'); - $this->assertFileExists($newPath, 'New path is created after write()'); + $file->write(); + $this->assertTrue( + $this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']), + 'Old path exists after draft change' + ); + $this->assertTrue( + $this->getAssetStore()->exists($newTuple['Filename'], $newTuple['Hash']), + 'New path is created after write()' + ); + + // After publish + $file->doPublish(); + $this->assertFalse( + $this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']), + 'Old file is finally removed after publishing new file' + ); + $this->assertTrue( + $this->getAssetStore()->exists($newTuple['Filename'], $newTuple['Hash']), + 'New path is created after write()' + ); } public function testSetParentIDChangesFilesystemOnWrite() { $file = $this->objFromFixture('File', 'asdf'); + $this->logInWithPermission('ADMIN'); + $file->doPublish(); $subfolder = $this->objFromFixture('Folder', 'subfolder'); - $oldPath = AssetStoreTest_SpyStore::getLocalPath($file); - $newPath = str_replace('assets/FileTest/', 'assets/FileTest/FileTest-subfolder/', $oldPath); + $oldTuple = $file->File->getValue(); // set ParentID $file->ParentID = $subfolder->ID; + $newTuple = $oldTuple; + $newTuple['Filename'] = $file->getFilename(); // Before write() - $this->assertFileExists($oldPath, 'Old path is still present'); - $this->assertFileNotExists($newPath, 'New path is updated in memory, not written before write() is called'); - $this->assertEquals($oldPath, AssetStoreTest_SpyStore::getLocalPath($file), 'URL is not updated until write is called'); - + $this->assertTrue( + $this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']), + 'Old path is still present' + ); + $this->assertFalse( + $this->getAssetStore()->exists($newTuple['Filename'], $newTuple['Hash']), + 'New path is updated in memory, not written before write() is called' + ); $file->write(); // After write() - $this->assertFileExists($oldPath, 'Old path is left after write()'); - $this->assertFileExists($newPath, 'New path is created after write()'); - $this->assertEquals($newPath, AssetStoreTest_SpyStore::getLocalPath($file), 'URL is updated after write is called'); + $file->write(); + $this->assertTrue( + $this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']), + 'Old path exists after draft change' + ); + $this->assertTrue( + $this->getAssetStore()->exists($newTuple['Filename'], $newTuple['Hash']), + 'New path is created after write()' + ); + + // After publish + $file->doPublish(); + $this->assertFalse( + $this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']), + 'Old file is finally removed after publishing new file' + ); + $this->assertTrue( + $this->getAssetStore()->exists($newTuple['Filename'], $newTuple['Hash']), + 'New path is created after write()' + ); } /** @@ -354,13 +409,29 @@ class FileTest extends SapphireTest { public function testDeleteFile() { $file = $this->objFromFixture('File', 'asdf'); - $fileID = $file->ID; - $filePath = AssetStoreTest_SpyStore::getLocalPath($file); - $file->delete(); + $this->logInWithPermission('ADMIN'); + $file->doPublish(); + $tuple = $file->File->getValue(); - // File is deleted - $this->assertFileNotExists($filePath); - $this->assertEmpty(DataObject::get_by_id('File', $fileID)); + // Before delete + $this->assertTrue( + $this->getAssetStore()->exists($tuple['Filename'], $tuple['Hash']), + 'File is still present' + ); + + // after unpublish + $file->doUnpublish(); + $this->assertTrue( + $this->getAssetStore()->exists($tuple['Filename'], $tuple['Hash']), + 'File is still present after unpublish' + ); + + // after delete + $file->delete(); + $this->assertFalse( + $this->getAssetStore()->exists($tuple['Filename'], $tuple['Hash']), + 'File is deleted after unpublish and delete' + ); } public function testRenameFolder() { @@ -462,6 +533,13 @@ class FileTest extends SapphireTest { $this->assertEquals('', File::join_paths('/', '/')); } + /** + * @return AssetStore + */ + protected function getAssetStore() { + return Injector::inst()->get('AssetStore'); + } + } class FileTest_MyCustomFile extends File implements TestOnly { diff --git a/tests/filesystem/FolderTest.php b/tests/filesystem/FolderTest.php index 1b1aba461..ff45d0fca 100644 --- a/tests/filesystem/FolderTest.php +++ b/tests/filesystem/FolderTest.php @@ -15,6 +15,9 @@ class FolderTest extends SapphireTest { public function setUp() { parent::setUp(); + $this->logInWithPermission('ADMIN'); + Versioned::reading_stage('Stage'); + // Set backend root to /FolderTest AssetStoreTest_SpyStore::activate('FolderTest'); @@ -123,7 +126,7 @@ class FolderTest extends SapphireTest { // File should be located in new folder $this->assertEquals( - ASSETS_PATH . '/FolderTest/FileTest-folder2/FileTest-folder1/55b443b601/File1.txt', + ASSETS_PATH . '/FolderTest/.protected/FileTest-folder2/FileTest-folder1/55b443b601/File1.txt', AssetStoreTest_SpyStore::getLocalPath($file1) ); } @@ -153,7 +156,7 @@ class FolderTest extends SapphireTest { // File should be located in new folder $this->assertEquals( - ASSETS_PATH . '/FolderTest/FileTest-folder1-changed/55b443b601/File1.txt', + ASSETS_PATH . '/FolderTest/.protected/FileTest-folder1-changed/55b443b601/File1.txt', AssetStoreTest_SpyStore::getLocalPath($file1) ); } diff --git a/tests/filesystem/UploadTest.php b/tests/filesystem/UploadTest.php index 6758cef1f..fa1202142 100644 --- a/tests/filesystem/UploadTest.php +++ b/tests/filesystem/UploadTest.php @@ -10,6 +10,7 @@ class UploadTest extends SapphireTest { public function setUp() { parent::setUp(); + Versioned::reading_stage('Stage'); AssetStoreTest_SpyStore::activate('UploadTest'); } @@ -48,7 +49,7 @@ class UploadTest extends SapphireTest { $file1->getFilename() ); $this->assertEquals( - BASE_PATH . '/assets/UploadTest/Uploads/315ae4c3d4/UploadTest-testUpload.txt', + BASE_PATH . '/assets/UploadTest/.protected/Uploads/315ae4c3d4/UploadTest-testUpload.txt', AssetStoreTest_SpyStore::getLocalPath($file1) ); $this->assertFileExists( @@ -66,7 +67,7 @@ class UploadTest extends SapphireTest { $file2->getFilename() ); $this->assertEquals( - BASE_PATH . '/assets/UploadTest/UploadTest-testUpload/315ae4c3d4/UploadTest-testUpload.txt', + BASE_PATH . '/assets/UploadTest/.protected/UploadTest-testUpload/315ae4c3d4/UploadTest-testUpload.txt', AssetStoreTest_SpyStore::getLocalPath($file2) ); $this->assertFileExists( diff --git a/tests/forms/uploadfield/AssetFieldTest.php b/tests/forms/uploadfield/AssetFieldTest.php index c61a18496..83d36a79b 100644 --- a/tests/forms/uploadfield/AssetFieldTest.php +++ b/tests/forms/uploadfield/AssetFieldTest.php @@ -15,6 +15,9 @@ class AssetFieldTest extends FunctionalTest { public function setUp() { parent::setUp(); + $this->logInWithPermission('ADMIN'); + Versioned::reading_stage('Stage'); + // Set backend root to /AssetFieldTest AssetStoreTest_SpyStore::activate('AssetFieldTest'); $create = function($path) { @@ -57,7 +60,7 @@ class AssetFieldTest extends FunctionalTest { $this->assertEquals('315ae4c3d44412baa0c81515b6fb35829a337a5a', $responseJSON[0]['hash']); $this->assertEmpty($responseJSON[0]['variant']); $this->assertFileExists( - BASE_PATH . '/assets/AssetFieldTest/MyDocuments/315ae4c3d4/testUploadBasic.txt' + BASE_PATH . '/assets/AssetFieldTest/.protected/MyDocuments/315ae4c3d4/testUploadBasic.txt' ); } @@ -79,7 +82,7 @@ class AssetFieldTest extends FunctionalTest { $responseJSON = json_decode($response->getBody(), true); $this->assertFalse($response->isError()); $this->assertFileExists( - BASE_PATH . '/assets/AssetFieldTest/MyFiles/315ae4c3d4/testUploadHasOneRelation.txt' + BASE_PATH . '/assets/AssetFieldTest/.protected/MyFiles/315ae4c3d4/testUploadHasOneRelation.txt' ); // Secondly, ensure that simply uploading an object does not save the file against the relation @@ -148,8 +151,7 @@ class AssetFieldTest extends FunctionalTest { $this->assertFalse($record->File->exists()); // Check file object itself exists - // @todo - When assets are removed from a DBFile reference, these files should be archived - $this->assertFileExists($filePath, 'File is only detached, not deleted from filesystem'); + $this->assertFileNotExists($filePath, 'File is deleted once detached'); } /** @@ -248,6 +250,7 @@ class AssetFieldTest extends FunctionalTest { } public function testCanUploadWithPermissionCode() { + Session::clear("loggedInAs"); $field = AssetField::create('MyField'); $field->setCanUpload(true); diff --git a/tests/forms/uploadfield/UploadFieldTest.php b/tests/forms/uploadfield/UploadFieldTest.php index 07ab27d1b..d95d605f8 100644 --- a/tests/forms/uploadfield/UploadFieldTest.php +++ b/tests/forms/uploadfield/UploadFieldTest.php @@ -14,9 +14,17 @@ class UploadFieldTest extends FunctionalTest { 'File' => array('UploadFieldTest_FileExtension') ); + protected $oldReadingMode = null; + public function setUp() { parent::setUp(); + $this->loginWithPermission('ADMIN'); + + // Save versioned state + $this->oldReadingMode = Versioned::get_reading_mode(); + Versioned::reading_stage('Stage'); + // Set backend root to /UploadFieldTest AssetStoreTest_SpyStore::activate('UploadFieldTest'); @@ -39,6 +47,9 @@ class UploadFieldTest extends FunctionalTest { public function tearDown() { AssetStoreTest_SpyStore::reset(); + if($this->oldReadingMode) { + Versioned::set_reading_mode($this->oldReadingMode); + } parent::tearDown(); } @@ -46,14 +57,13 @@ class UploadFieldTest extends FunctionalTest { * Test that files can be uploaded against an object with no relation */ public function testUploadNoRelation() { - $this->loginWithPermission('ADMIN'); - $tmpFileName = 'testUploadBasic.txt'; $response = $this->mockFileUpload('NoRelationField', $tmpFileName); $this->assertFalse($response->isError()); $uploadedFile = DataObject::get_one('File', array( '"File"."Name"' => $tmpFileName )); + $this->assertFileExists(AssetStoreTest_SpyStore::getLocalPath($uploadedFile)); $this->assertTrue(is_object($uploadedFile), 'The file object is created'); } @@ -62,8 +72,6 @@ class UploadFieldTest extends FunctionalTest { * Test that an object can be uploaded against an object with a has_one relation */ public function testUploadHasOneRelation() { - $this->loginWithPermission('ADMIN'); - // Unset existing has_one relation before re-uploading $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $record->HasOneFileID = null; @@ -95,8 +103,6 @@ class UploadFieldTest extends FunctionalTest { * Tests that has_one relations work with subclasses of File */ public function testUploadHasOneRelationWithExtendedFile() { - $this->loginWithPermission('ADMIN'); - // Unset existing has_one relation before re-uploading $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $record->HasOneExtendedFileID = null; @@ -129,8 +135,6 @@ class UploadFieldTest extends FunctionalTest { * Test that has_many relations work with files */ public function testUploadHasManyRelation() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); // Test that uploaded files can be posted to a has_many relation @@ -159,8 +163,6 @@ class UploadFieldTest extends FunctionalTest { * Test that many_many relationships work with files */ public function testUploadManyManyRelation() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $relationCount = $record->ManyManyFiles()->Count(); @@ -195,8 +197,6 @@ class UploadFieldTest extends FunctionalTest { * in this controller method. */ public function testAllowedExtensions() { - $this->loginWithPermission('ADMIN'); - // Test invalid file // Relies on Upload_Validator failing to allow this extension $invalidFile = 'invalid.php'; @@ -237,8 +237,6 @@ class UploadFieldTest extends FunctionalTest { * Test that has_one relations do not support multiple files */ public function testAllowedMaxFileNumberWithHasOne() { - $this->loginWithPermission('ADMIN'); - // Get references for each file to upload $file1 = $this->objFromFixture('File', 'file1'); $file2 = $this->objFromFixture('File', 'file2'); @@ -272,8 +270,6 @@ class UploadFieldTest extends FunctionalTest { * Test that max number of items on has_many is validated */ public function testAllowedMaxFileNumberWithHasMany() { - $this->loginWithPermission('ADMIN'); - // The 'HasManyFilesMaxTwo' field has a maximum of two files able to be attached to it. // We want to add files to it until we attempt to add the third. We expect that the first // two should work and the third will fail. @@ -407,8 +403,6 @@ class UploadFieldTest extends FunctionalTest { * Test that files can be deleted from has_one */ public function testDeleteFromHasOne() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file1 = $this->objFromFixture('File', 'file1'); @@ -431,8 +425,6 @@ class UploadFieldTest extends FunctionalTest { * Test that files can be deleted from has_many */ public function testDeleteFromHasMany() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file2 = $this->objFromFixture('File', 'file2'); $file3 = $this->objFromFixture('File', 'file3'); @@ -457,8 +449,6 @@ class UploadFieldTest extends FunctionalTest { * Test that files can be deleted from many_many and the filesystem */ public function testDeleteFromManyMany() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file4 = $this->objFromFixture('File', 'file4'); $file5 = $this->objFromFixture('File', 'file5'); @@ -496,8 +486,6 @@ class UploadFieldTest extends FunctionalTest { * Test control output html */ public function testView() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file4 = $this->objFromFixture('File', 'file4'); $file5 = $this->objFromFixture('File', 'file5'); @@ -523,8 +511,6 @@ class UploadFieldTest extends FunctionalTest { } public function testEdit() { - $memberID = $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file4 = $this->objFromFixture('File', 'file4'); $fileNoEdit = $this->objFromFixture('File', 'file-noedit'); @@ -630,8 +616,6 @@ class UploadFieldTest extends FunctionalTest { } public function testReadonly() { - $this->loginWithPermission('ADMIN'); - $response = $this->get('UploadFieldTest_Controller'); $this->assertFalse($response->isError()); @@ -655,8 +639,6 @@ class UploadFieldTest extends FunctionalTest { } public function testDisabled() { - $this->loginWithPermission('ADMIN'); - $response = $this->get('UploadFieldTest_Controller'); $this->assertFalse($response->isError()); @@ -677,7 +659,6 @@ class UploadFieldTest extends FunctionalTest { } public function testCanUpload() { - $this->loginWithPermission('ADMIN'); $response = $this->get('UploadFieldTest_Controller'); $this->assertFalse($response->isError()); @@ -697,6 +678,7 @@ class UploadFieldTest extends FunctionalTest { public function testCanUploadWithPermissionCode() { $field = UploadField::create('MyField'); + Session::clear("loggedInAs"); $field->setCanUpload(true); $this->assertTrue($field->canUpload()); @@ -714,7 +696,6 @@ class UploadFieldTest extends FunctionalTest { } public function testCanAttachExisting() { - $this->loginWithPermission('ADMIN'); $response = $this->get('UploadFieldTest_Controller'); $this->assertFalse($response->isError()); @@ -740,8 +721,6 @@ class UploadFieldTest extends FunctionalTest { } public function testSelect() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file4 = $this->objFromFixture('File', 'file4'); $fileSubfolder = $this->objFromFixture('File', 'file-subfolder'); @@ -758,8 +737,6 @@ class UploadFieldTest extends FunctionalTest { } public function testSelectWithDisplayFolderName() { - $this->loginWithPermission('ADMIN'); - $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file4 = $this->objFromFixture('File', 'file4'); $fileSubfolder = $this->objFromFixture('File', 'file-subfolder'); @@ -779,8 +756,6 @@ class UploadFieldTest extends FunctionalTest { * Test that UploadField:overwriteWarning cannot overwrite Upload:replaceFile */ public function testConfigOverwriteWarningCannotRelaceFiles() { - $this->loginWithPermission('ADMIN'); - Upload::config()->replaceFile = false; UploadField::config()->defaultConfig = array_merge( UploadField::config()->defaultConfig, array('overwriteWarning' => true) @@ -815,8 +790,6 @@ class UploadFieldTest extends FunctionalTest { * Tests that UploadField::fileexist works */ public function testFileExists() { - $this->loginWithPermission('ADMIN'); - // Check that fileexist works on subfolders $nonFile = uniqid().'.txt'; $responseEmpty = $this->mockFileExists('NoRelationField', $nonFile); @@ -834,7 +807,7 @@ class UploadFieldTest extends FunctionalTest { $tmpFileName = 'testUploadBasic.txt'; $response = $this->mockFileUpload('RootFolderTest', $tmpFileName); $this->assertFalse($response->isError()); - $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/315ae4c3d4/$tmpFileName"); + $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/.protected/315ae4c3d4/$tmpFileName"); $responseExists = $this->mockFileExists('RootFolderTest', $tmpFileName); $responseExistsData = json_decode($responseExists->getBody()); $this->assertFalse($responseExists->isError()); @@ -843,7 +816,7 @@ class UploadFieldTest extends FunctionalTest { // Check that uploaded files can be detected $response = $this->mockFileUpload('NoRelationField', $tmpFileName); $this->assertFalse($response->isError()); - $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/UploadedFiles/315ae4c3d4/$tmpFileName"); + $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/.protected/UploadedFiles/315ae4c3d4/$tmpFileName"); $responseExists = $this->mockFileExists('NoRelationField', $tmpFileName); $responseExistsData = json_decode($responseExists->getBody()); $this->assertFalse($responseExists->isError()); @@ -855,7 +828,7 @@ class UploadFieldTest extends FunctionalTest { $tmpFileNameExpected = 'test-Upload-Bad.txt'; $response = $this->mockFileUpload('NoRelationField', $tmpFileName); $this->assertFalse($response->isError()); - $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/UploadedFiles/315ae4c3d4/$tmpFileNameExpected"); + $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/.protected/UploadedFiles/315ae4c3d4/$tmpFileNameExpected"); // With original file $responseExists = $this->mockFileExists('NoRelationField', $tmpFileName); $responseExistsData = json_decode($responseExists->getBody()); @@ -869,7 +842,6 @@ class UploadFieldTest extends FunctionalTest { // Test that attempts to navigate outside of the directory return false $responseExists = $this->mockFileExists('NoRelationField', "../../../../var/private/$tmpFileName"); - $responseExistsData = json_decode($responseExists->getBody()); $this->assertTrue($responseExists->isError()); $this->assertContains('File is not a valid upload', $responseExists->getBody()); } @@ -922,6 +894,7 @@ class UploadFieldTest extends FunctionalTest { $form = new UploadFieldTestForm(); $form->loadDataFrom($data, true); + if($form->validate()) { $record = $form->getRecord(); $form->saveInto($record); @@ -996,6 +969,35 @@ class UploadFieldTest extends FunctionalTest { ); } + public function get($url, $session = null, $headers = null, $cookies = null) { + // Inject stage=Stage into the URL, to force working on draft + $url = $this->addStageToUrl($url); + return parent::get($url, $session, $headers, $cookies); + } + + public function post($url, $data, $headers = null, $session = null, $body = null, $cookies = null) { + // Inject stage=Stage into the URL, to force working on draft + $url = $this->addStageToUrl($url); + return parent::post($url, $data, $headers, $session, $body, $cookies); + } + + /** + * Adds ?stage=Stage to url + * + * @param string $url + * @return string + */ + protected function addStageToUrl($url) { + if(stripos($url, 'stage=Stage') === false) { + if(stripos($url, '?') === false) { + $url .= '?stage=Stage'; + } else { + $url .= '&stage=Stage'; + } + } + return $url; + } + } class UploadFieldTest_Record extends DataObject implements TestOnly { diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php index 803c23126..0e6299b86 100644 --- a/tests/model/VersionedTest.php +++ b/tests/model/VersionedTest.php @@ -102,6 +102,22 @@ class VersionedTest extends SapphireTest { $this->assertEquals($count, $count2); } + /** + * Test that publishing from invalid stage will throw exception + */ + public function testInvalidPublish() { + $obj = new VersionedTest_Subclass(); + $obj->ExtraField = 'Foo'; // ensure that child version table gets written + $obj->write(); + $this->setExpectedException( + 'InvalidArgumentException', + "Can't find VersionedTest_DataObject#{$obj->ID} in stage Live" + ); + + // Fail publishing from live to stage + $obj->publish('Live', 'Stage'); + } + public function testDuplicate() { $obj1 = new VersionedTest_Subclass(); $obj1->ExtraField = 'Foo';