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

@@ -0,0 +1,6 @@
+---
+Name: versioning
+---
+GridFieldDetailForm:
+  extensions:
+    - VersionedGridFieldDetailForm

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) {
 

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

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
+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 deletion of 
+will automatically have their attached assets moved to the protected store. The archive of 
-draft or unversioned objects will have those assets permanantly deleted (along with all variants).
+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,
+Note that regardless of this setting, the database record will still be archived in the
-rather than deleted. This uses more disk storage, but will allow the full recovery of archived
+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 */
+        /** Ensure assets are archived along with the DataObject */
-        private static $archive_assets = true;
+        private static $keep_archived_assets = true;
         /** Versioned */
         private static $extensions = array('Versioned');
     }

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
 

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?
+	 * When archiving versioned dataobjects, should assets be archived with them?
-     * If false, assets will be deleted when the object is removed from draft.
+	 * If false, assets will be deleted when the dataobject is archived.
-     * If true, assets will be instead moved to the protected store.
+	 * If true, assets will be instead moved to the protected store, and can be
-     *
+	 * restored when the dataobject is restored from archive.
-     * @var bool
+	 *
-     */
+	 * Note that this does not affect the archiving of the actual database record in any way,
-    private static $archive_assets = false;
+	 * 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
+		// Add all assets for deletion
-        // Note that DataObject::delete() ignores sourceQueryParams
+		$this->addAssetsFromRecord($manipulations, $this->owner, AssetManipulationList::STATE_DELETED);
-        if($this->isVersioned() && \Versioned::current_stage() === \Versioned::get_live_stage()) {
-            $this->protectAll($assets);
-            return;
-        }
 
-        // When deleting from stage then check if we should archive assets
+		// Whitelist assets that exist in other stages
-        $archive = $this->owner->config()->archive_assets;
+		$this->addAssetsFromOtherStages($manipulations);
-        if($archive && $this->isVersioned()) {
-            // Archived assets are kept protected
-            $this->protectAll($assets);
-        } else {
-            // Otherwise remove all assets
-            $this->deleteAll($assets);
-        }
-    }
 
-    /**
+		// Apply visibility rules based on the final manipulation
-     * Return a list of all tuples attached to this dataobject
+		$this->processManipulation($manipulations);
-     * 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();
+	 * Ensure that changes to records flush overwritten files, and update the visibility
-            unset($next['Variant']);
+	 * of other assets.
-            if ($next) {
+	 */
-                $files[] = $next;
+	public function onBeforeWrite()
-            }
+	{
-        }
+		// Prepare blank manipulation
+		$manipulations = new AssetManipulationList();
 
-        // De-dupe
+		// Mark overwritten object as deleted
-        return array_map("unserialize", array_unique(array_map("serialize", $files)));
+		if($this->owner->isInDB()) {
-    }
+			$priorRecord = DataObject::get(get_class($this->owner))->byID($this->owner->ID);
+			if($priorRecord) {
+				$this->addAssetsFromRecord($manipulations, $priorRecord, AssetManipulationList::STATE_DELETED);
+			}
+		}
 
-    /**
+		// Add assets from new record with the correct visibility rules
-     * Determine if versioning rules should be applied to this object
+		$state = $this->getRecordState($this->owner);
-     *
+		$this->addAssetsFromRecord($manipulations, $this->owner, $state);
-     * @return bool
-     */
-    protected function isVersioned() {
-        return $this->owner->has_extension('Versioned');
-    }
 
-    /**
+		// Whitelist assets that exist in other stages
-     * Delete all assets in the tuple list
+		$this->addAssetsFromOtherStages($manipulations);
-     *
-     * @param array $assets
-     */
-    protected function deleteAll($assets) {
-        $store = $this->getAssetStore();
-        foreach($assets as $asset) {
-            $store->delete($asset['Filename'], $asset['Hash']);
-        }
-    }
 
-    /**
+		// Apply visibility rules based on the final manipulation
-     * Move all assets in the list to the protected store
+		$this->processManipulation($manipulations);
-     *
+	}
-     * @param array $assets
-     */
-    protected function protectAll($assets) {
-        $store = $this->getAssetStore();
-        foreach($assets as $asset) {
-            $store->protect($asset['Filename'], $asset['Hash']);
-        }
-    }
 
-    /**
+	/**
-     * @return AssetStore
+	 * Check default state of this record
-     */
+	 *
-    protected function getAssetStore() {
+	 * @param DataObject $record
-        return Injector::inst()->get('AssetStore');
+	 * @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');
+	}
 }

filesystem/AssetManipulationList.php

@@ -0,0 +1,175 @@
+<?php
+
+namespace SilverStripe\Filesystem;
+
+/**
+ * Provides a mechanism for determining the effective visibility of a set of assets (identified by
+ * filename and hash), given their membership to objects of varying visibility.
+ *
+ * The effective visibility of assets is based on three rules:
+ * - If an asset is attached to any public record, that asset is public.
+ * - If an asset is not attached to any public record, but is attached to a protected record,
+ *   that asset is protected.
+ * - If an asset is attached to a record being deleted, but not any existing public or protected
+ *   record, then that asset is marked for deletion.
+ *
+ * Variants are ignored for the purpose of determining visibility
+ */
+class AssetManipulationList
+{
+
+    const STATE_PUBLIC = 'public';
+
+    const STATE_PROTECTED = 'protected';
+
+    const STATE_DELETED = 'deleted';
+
+    /**
+     * List of public assets
+     *
+     * @var array
+     */
+    protected $public = array();
+
+    /**
+     * List of protected assets
+     *
+     * @var array
+     */
+    protected $protected = array();
+
+    /**
+     * List of deleted assets
+     *
+     * @var array
+     */
+    protected $deleted = array();
+
+    /**
+     * Get an identifying key for a given filename and hash
+     *
+     * @param array $asset Asset tuple
+     * @return string
+     */
+    protected function getAssetKey($asset)
+    {
+        return $asset['Hash'] . '/' . $asset['Filename'];
+    }
+
+    /**
+     * Add asset with the given state
+     *
+     * @param array $asset Asset tuple
+     * @param string $state One of the STATE_* const vars
+     * @return bool True if the asset was added to the set matching the given state
+     */
+    public function addAsset($asset, $state)
+    {
+        switch ($state) {
+            case self::STATE_PUBLIC:
+                return $this->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;
+    }
+}

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",
+		"Parent" => "File",
-		"Owner" =>"Member"
+		"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')) {
+			$errorCode = 404;
-				$record = ErrorPage::get()->filter("ErrorCode", 404)->first();
+		} elseif(!$record->canView()) {
+			$errorCode = 403;
+		}
+		if($errorCode) {
+			$result = static::singleton()->invokeWithExtensions('getErrorRecordFor', $errorCode);
+			$result = array_filter($result);
+			if($result) {
+				$record = reset($result);
 			}
+		}
 
-			if (!$record) {
+		if (!$record) {
-				return; // There were no suitable matches at all.
+			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('<a href="%s" target="_blank">%s</a>', $this->Link(), $this->RelativeLink())
+						sprintf('<a href="%s" target="_blank">%s</a>', $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.
 	 *

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;
 	}
 

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) {
+	public function setReplaceFile($replace) {
-		$this->replaceFile = $bool;
+		$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);

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');

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);

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)) {

forms/gridfield/GridFieldDetailForm.php

@@ -1,4 +1,5 @@
 <?php
+use SilverStripe\Framework\Core\Extensible;
 
 /**
  * Provides view and edit forms at GridField-specific URLs.
@@ -18,8 +19,10 @@
  */
 class GridFieldDetailForm implements GridField_URLHandler {
 
+	use Extensible;
+
 	/**
-	 * @var String
+	 * @var string
 	 */
 	protected $template = 'GridFieldDetailForm';
 
@@ -40,12 +43,12 @@ class GridFieldDetailForm implements GridField_URLHandler {
 	protected $fields;
 
 	/**
-	 * @var String
+	 * @var string
 	 */
 	protected $itemRequestClass;
 
 	/**
-	 * @var function With two parameters: $form and $component
+	 * @var callable With two parameters: $form and $component
 	 */
 	protected $itemEditFormCallback;
 
@@ -68,6 +71,7 @@ class GridFieldDetailForm implements GridField_URLHandler {
 	 */
 	public function __construct($name = 'DetailForm') {
 		$this->name = $name;
+		$this->constructExtensions();
 	}
 
 	/**
@@ -88,10 +92,7 @@ class GridFieldDetailForm implements GridField_URLHandler {
 			$record = Object::create($gridField->getModelClass());
 		}
 
-		$class = $this->getItemRequestClass();
+		$handler = $this->getItemRequestHandler($gridField, $record, $requestHandler);
-
-		$handler = Object::create($class, $gridField, $this, $record, $requestHandler, $this->name);
-		$handler->setTemplate($this->template);
 
 		// 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();
+		// Build actions
-		if($this->record->ID !== 0) {
+		$actions = $this->getFormActions();
-			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(
-					"<a class=\"%s\" href=\"%s\">%s</a>",
-					"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));
-			}
-		}
 
 		$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(
+					"<a class=\"%s\" href=\"%s\">%s</a>",
+					"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;
+		$isNewRecord = $this->record->ID == 0;
-		$controller = $this->getToplevelController();
-		$list = $this->gridField->getList();
 
-		if(!$this->record->canEdit()) {
+		// Check permission
-			return $controller->httpError(403);
+		if (!$this->record->canEdit()) {
-		}
+			return $this->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);
 		}
 
+		// Save from form data
 		try {
-			$form->saveInto($this->record);
+			$this->saveFormIntoRecord($data, $form);
-			$this->record->write();
+		} catch (ValidationException $e) {
-			$extraData = $this->getExtraSavedData($this->record, $list);
+			return $this->generateValidationResponse($form, $e);
-			$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());
 		}
 
-		// TODO Save this item into the given relationship
-
 		$link = '<a href="' . $this->Link('edit') . '">"'
 			. htmlspecialchars($this->record->Title, ENT_QUOTES)
 			. '"</a>';
@@ -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 {

model/DataExtension.php

@@ -2,6 +2,8 @@
 /**
  * An extension that adds additional functionality to a {@link DataObject}.
  *
+ * @property DataObject $owner
+ *
  * @package framework
  * @subpackage model
  */

model/versioning/Versioned.php

@@ -1,5 +1,7 @@
 <?php
 
+// namespace SilverStripe\Framework\Model\Versioning
+
 /**
  * The Versioned extension allows your DataObjects to have several versions,
  * allowing you to rollback changes and view history. An example of this is
@@ -739,13 +741,116 @@ class Versioned extends DataExtension implements TemplateGlobalProvider {
 	/**
 	 * If a write was skipped, then we need to ensure that we don't leave a
 	 * migrateVersion() value lying around for the next write.
-	 *
-	 *
 	 */
 	public function onAfterSkippedWrite() {
 		$this->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();
+			$owner->flushCache();
-			$from = Versioned::get_one_by_stage($baseClass, $fromStage, "\"{$baseClass}\".\"ID\"={$this->owner->ID}");
+			$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();
-			$from->forceChange();
+		if($createNewVersion) {
-			if($createNewVersion) {
+			$latest = self::get_latest_version($baseClass, $owner->ID);
-				$latest = self::get_latest_version($baseClass, $this->owner->ID);
+			$owner->Version = $latest->Version + 1;
-				$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);
 		} 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);
-
+		$owner->writeWithoutVersion();
-		$this->owner->writeWithoutVersion();
+		$owner->extend('onAfterRollback', $version);
-
-		$this->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');
 	}
 
 	/**

model/versioning/VersionedGridFieldDetailForm.php

@@ -0,0 +1,13 @@
+<?php
+
+/**
+ * Extends {@see GridFieldDetailForm}
+ */
+class VersionedGridFieldDetailForm extends Extension {
+    public function updateItemRequestClass(&$class, $gridField, $record, $requestHandler) {
+        // Conditionally use a versioned item handler
+        if($record && $record->has_extension('Versioned')) {
+            $class = 'VersionedGridFieldItemRequest';
+        }
+    }
+}

model/versioning/VersionedGridFieldItemRequest.php

@@ -0,0 +1,196 @@
+<?php
+
+/**
+ * Provides versioned dataobject support to {@see GridFieldDetailForm_ItemRequest}
+ *
+ * @property GridFieldDetailForm_ItemRequest $owner
+ */
+class VersionedGridFieldItemRequest extends GridFieldDetailForm_ItemRequest {
+
+    protected function getFormActions() {
+        $actions = parent::getFormActions();
+
+		// Check if record is versionable
+		$record = $this->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 = "<a href=\"{$editURL}\">{$xmlTitle}</a>";
+		$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);
+		}
+	}
+}

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
 	 * 

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");
 		}
 	}
 

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;
 }

tests/filesystem/AssetManipulationListTest.php

@@ -0,0 +1,79 @@
+<?php
+use SilverStripe\Filesystem\AssetManipulationList;
+
+/**
+ * Tests set manipulations of groups of assets of differing visibilities
+ */
+class AssetManipulationListTest extends SapphireTest {
+
+    public function testVisibility() {
+        $set = new AssetManipulationList();
+        $file1 = ['Filename' => '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());
+    }
+}

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");
 		}
 	}
 

tests/filesystem/FileTest.php

@@ -1,6 +1,7 @@
 <?php
 
 use Filesystem as SS_Filesystem;
+use SilverStripe\Filesystem\Storage\AssetStore;
 
 /**
  * Tests for the File class
@@ -13,6 +14,8 @@ class FileTest extends SapphireTest {
 
 	public function setUp() {
 		parent::setUp();
+		$this->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);
+		$this->logInWithPermission('ADMIN');
-		$newPath = str_replace('FileTest.txt', 'renamed.txt', $oldPath);
+		$file->doPublish();
+		$oldTuple = $file->File->getValue();
+
+		// Rename
+		$file->Name = 'renamed.txt';
+		$newTuple = $oldTuple;
+		$newTuple['Filename'] = $file->getFilename();
 
 		// Before write()
-		$file->Name = 'renamed.txt';
+		$this->assertTrue(
-		$this->assertFileExists($oldPath, 'Old path is still present');
+			$this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']),
-		$this->assertFileNotExists($newPath, 'New path is updated in memory, not written before write() is called');
+			'Old path is still present'
-		$file->write();
+		);
+		$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()');
+		$file->write();
-		$this->assertFileExists($newPath, 'New path is created after 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);
+		$oldTuple = $file->File->getValue();
-		$newPath = str_replace('assets/FileTest/', 'assets/FileTest/FileTest-subfolder/', $oldPath);
 
 		// set ParentID
 		$file->ParentID = $subfolder->ID;
+		$newTuple = $oldTuple;
+		$newTuple['Filename'] = $file->getFilename();
 
 		// Before write()
-		$this->assertFileExists($oldPath, 'Old path is still present');
+		$this->assertTrue(
-		$this->assertFileNotExists($newPath, 'New path is updated in memory, not written before write() is called');
+			$this->getAssetStore()->exists($oldTuple['Filename'], $oldTuple['Hash']),
-		$this->assertEquals($oldPath, AssetStoreTest_SpyStore::getLocalPath($file), 'URL is not updated until write is called');
+			'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()');
+		$file->write();
-		$this->assertFileExists($newPath, 'New path is created after write()');
+		$this->assertTrue(
-		$this->assertEquals($newPath, AssetStoreTest_SpyStore::getLocalPath($file), 'URL is updated after write is called');
+			$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;
+		$this->logInWithPermission('ADMIN');
-		$filePath = AssetStoreTest_SpyStore::getLocalPath($file);
+		$file->doPublish();
-		$file->delete();
+		$tuple = $file->File->getValue();
 
-		// File is deleted
+		// Before delete
-		$this->assertFileNotExists($filePath);
+		$this->assertTrue(
-		$this->assertEmpty(DataObject::get_by_id('File', $fileID));
+			$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 {

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)
 		);
 	}

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(

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->assertFileNotExists($filePath, 'File is deleted once detached');
-		$this->assertFileExists($filePath, 'File is only detached, not deleted from filesystem');
 	}
 
 	/**
@@ -248,6 +250,7 @@ class AssetFieldTest extends FunctionalTest {
 	}
 
 	public function testCanUploadWithPermissionCode() {
+		Session::clear("loggedInAs");
 		$field = AssetField::create('MyField');
 
 		$field->setCanUpload(true);

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 {

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';