Merge pull request #4679 from open-sausages/pulls/4.0/dbfile-file-dataobject

BUG Fix file link tracking for new asset abstraction

docs/en/04_Changelogs/4.0.0.md

@@ -15,6 +15,10 @@
    can be used to validate these types.
  * `Image_Backend` API now loads and saves from `AssetContainer` instances rather than local files.
  * The following File categories have been renamed: 'zip' to 'archive', 'doc' to 'document', and 'mov' to 'video'
+ * `File::updateLinks` no longer takes urls as parameters. All file links are now identified either by
+   the `DataObject::ID` in a `data-fileid` property, or via shortcodes. This is necessary because file
+   urls are no longer able to identify assets.
+ * Extension point `HtmlEditorField::processImage` has been removed, and moved to `Image::regenerateImageHTML`
 
 ## New API
 

filesystem/File.php

@@ -559,12 +559,15 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer {
 			$pathAfter = $result['Filename'];
 			$this->setFilename($pathAfter);
 		}
-
-		// Update any database references
-		$this->updateLinks($pathBefore, $pathAfter);
 		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
@@ -640,11 +643,12 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer {
 	}
 
 	/**
-	 * @param String $old File path relative to the webroot
-	 * @param String $new File path relative to the webroot
+	 * Trigger update of all links to this file
+	 *
+	 * If CMS Module is installed, {@see SiteTreeFileExtension::updateLinks}
 	 */
-	protected function updateLinks($old, $new) {
-		$this->extend('updateLinks', $old, $new);
+	protected function updateLinks() {
+		$this->extend('updateLinks');
 	}
 
 	/**

filesystem/storage/DBFile.php

@@ -15,7 +15,7 @@ use SilverStripe\Filesystem\Storage\AssetStore;
  * @property string $Variant Variant of the file
  *
  * @package framework
- * @subpackage model
+ * @subpackage filesystem
  */
 class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandler {
 

forms/HtmlEditorField.php

@@ -74,7 +74,9 @@ class HtmlEditorField extends TextareaField {
 			);
 		}
 
-		$htmlValue = Injector::inst()->create('HTMLValue', $this->value);
+		// Resample images
+		$value = Image::regenerate_html_links($this->value);
+		$htmlValue = Injector::inst()->create('HTMLValue', $value);
 
 		// Sanitise if requested
 		if($this->config()->sanitise_server_side) {
@@ -82,35 +84,6 @@ class HtmlEditorField extends TextareaField {
 			$santiser->sanitise($htmlValue);
 		}
 
-		// Resample images and add default attributes
-		if($images = $htmlValue->getElementsByTagName('img')) foreach($images as $img) {
-			// strip any ?r=n data from the src attribute
-			$img->setAttribute('src', preg_replace('/([^\?]*)\?r=[0-9]+$/i', '$1', $img->getAttribute('src')));
-
-			// Resample the images if the width & height have changed.
-			$fileID = $img->getAttribute('data-fileid');
-			if($fileID && ($image = File::get()->byID($fileID))) {
-				$width  = (int)$img->getAttribute('width');
-				$height = (int)$img->getAttribute('height');
-
-				if($width && $height && ($width != $image->getWidth() || $height != $image->getHeight())) {
-					//Make sure that the resized image actually returns an image:
-					$resized = $image->ResizedImage($width, $height);
-					if($resized) {
-						$img->setAttribute('src', $resized->getURL());
-					}
-				}
-			}
-
-			// Add default empty title & alt attributes.
-			if(!$img->getAttribute('alt')) $img->setAttribute('alt', '');
-			if(!$img->getAttribute('title')) $img->setAttribute('title', '');
-
-			// Use this extension point to manipulate images inserted using TinyMCE, e.g. add a CSS class, change default title
-			// $image is the image, $img is the DOM model
-			$this->extend('processImage', $image, $img);
-		}
-
 		// optionally manipulate the HTML after a TinyMCE edit and prior to a save
 		$this->extend('processHTML', $htmlValue);
 

model/Image.php

@@ -24,4 +24,62 @@ class Image extends File {
 	public function getIsImage() {
 		return true;
 	}
+
+	/**
+	 * Helper method to regenerate all image links in the given HTML block, optionally resizing them if
+	 * the image native size differs to the width and height properties on the <img /> tag
+	 *
+	 * @param string $value HTML value
+	 * @return string value with links resampled
+	 */
+	public static function regenerate_html_links($value) {
+		$htmlValue = Injector::inst()->create('HTMLValue', $value);
+
+		// Resample images and add default attributes
+		$imageElements = $htmlValue->getElementsByTagName('img');
+		if($imageElements) foreach($imageElements as $imageElement) {
+			$imageDO = null;
+			$src = $imageElement->getAttribute('src');
+
+			// Skip if this image has a shortcode 'src'
+			if(preg_match('/^\[.+\]$/', $src)) {
+				continue;
+			}
+
+			// strip any ?r=n data from the src attribute
+			$src = preg_replace('/([^\?]*)\?r=[0-9]+$/i', '$1', $src);
+
+			// Resample the images if the width & height have changed.
+			$fileID = $imageElement->getAttribute('data-fileid');
+			if($fileID && ($imageDO = File::get()->byID($fileID))) {
+				$width  = (int)$imageElement->getAttribute('width');
+				$height = (int)$imageElement->getAttribute('height');
+				if($imageDO instanceof Image && $width && $height
+					&& ($width != $imageDO->getWidth() || $height != $imageDO->getHeight())
+				) {
+					//Make sure that the resized image actually returns an image:
+					$resized = $imageDO->ResizedImage($width, $height);
+					if($resized) {
+						$imageDO = $resized;
+					}
+				}
+				$src = $imageDO->getURL();
+			}
+
+			// Update attributes, including intelligent defaults for alt and title
+			$imageElement->setAttribute('src', $src);
+			if(!$imageElement->getAttribute('alt')) {
+				$imageElement->setAttribute('alt', '');
+			}
+			if(!$imageElement->getAttribute('title')) {
+				$imageElement->setAttribute('title', '');
+			}
+
+			// Use this extension point to manipulate images inserted using TinyMCE,
+			// e.g. add a CSS class, change default title
+			self::singleton()
+				->extend('regenerateImageHTML', $imageDO, $imageElement);
+		}
+		return $htmlValue->getContent();
+	}
 }

tests/filesystem/AssetStoreTest.php

@@ -432,7 +432,18 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore {
 		self::$basedir = $basedir;
 
 		// Ensure basedir exists
-		SS_Filesystem::makeFolder(ASSETS_PATH . '/' . self::$basedir);
+		SS_Filesystem::makeFolder(self::base_path());
+	}
+
+	/**
+	 * Get absolute path to basedir
+	 *
+	 * @return string
+	 */
+	public static function base_path() {
+		if(self::$basedir) {
+			return ASSETS_PATH . '/' . self::$basedir;
+		}
 	}
 
 	/**
@@ -440,7 +451,7 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore {
 	 */
 	public static function reset() {
 		if(self::$basedir) {
-			$path = ASSETS_PATH . '/' . self::$basedir;
+			$path = self::base_path();
 			if(file_exists($path)) {
 				SS_Filesystem::removeFolder($path);
 			}
@@ -456,7 +467,7 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore {
 	 */
 	public static function getLocalPath(AssetContainer $asset) {
 		if($asset instanceof Folder) {
-			return ASSETS_PATH . '/' . self::$basedir . '/' . $asset->getFilename();
+			return self::base_path() . '/' . $asset->getFilename();
 		}
 		return BASE_PATH . $asset->getUrl();
 	}