API Update link tracking for image shortcodes

code/model/SiteTree.php

@@ -1612,46 +1612,6 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		return ($liveRecord) ? $liveRecord->URLSegment : null;
 	}
 
-	/**
-	 * Rewrites any linked images on this page without creating a new version record.
-	 * Non-image files should be linked via shortcodes
-	 * Triggers the onRenameLinkedAsset action on extensions.
-	 *
-	 * @todo Implement image shortcodes and remove this feature
-	 */
-	public function rewriteFileLinks() {
-		// Skip live stage
-		if(\Versioned::get_stage() === \Versioned::LIVE) {
-			return;
-		}
-
-		// Update the content without actually creating a new version
-		foreach($this->db() as $fieldName => $fieldType) {
-			// Skip if non HTML or if empty
-			if ($fieldType !== 'HTMLText') {
-				continue;
-			}
-			$fieldValue = $this->{$fieldName};
-			if(empty($fieldValue)) {
-				continue;
-			}
-
-			// Regenerate content
-			$content = Image::regenerate_html_links($fieldValue);
-			if($content === $fieldValue) {
-				continue;
-			}
-
-			// Write content directly without updating linked assets
-			$table = ClassInfo::table_for_object_field($this, $fieldName);
-			$query = sprintf('UPDATE "%s" SET "%s" = ? WHERE "ID" = ?', $table, $fieldName);
-			DB::prepared_query($query, array($content, $this->ID));
-
-			// Update linked assets
-			$this->invokeWithExtensions('onRenameLinkedAsset');
-		}
-	}
-
 	/**
 	 * Returns the pages that depend on this page. This includes virtual pages, pages that link to it, etc.
 	 *

code/model/SiteTreeFileExtension.php

@@ -124,43 +124,4 @@ class SiteTreeFileExtension extends DataExtension {
 			}
 		}
 	}
-
-	public function onAfterWrite() {
-		// Update any database references in the current stage
-		$this->updateLinks();
-	}
-
-	public function onAfterVersionedPublish() {
-		// Ensure that ->updateLinks is invoked on the draft record
-		// after ->doPublish() is invoked.
-		$this->updateLinks();
-	}
-
-	/**
-	 * Rewrite links to the $old file to now point to the $new file.
-	 *
-	 * @uses SiteTree->rewriteFileID()
-	 */
-	public function updateLinks() {
-		// Skip live stage
-		if(\Versioned::get_stage() === \Versioned::LIVE) {
-			return;
-		}
-
-		if(class_exists('Subsite')) {
-			Subsite::disable_subsite_filter(true);
-		}
-
-		$pages = $this->owner->BackLinkTracking();
-		if($pages) {
-			foreach($pages as $page) {
-				$page->rewriteFileLinks();
-			}
-		}
-
-		if(class_exists('Subsite')) {
-			Subsite::disable_subsite_filter(false);
-		}
-	}
-
 }

code/model/SiteTreeLinkTracking.php

@@ -93,8 +93,6 @@ class SiteTreeLinkTracking extends DataExtension {
 	/**
 	 * Scrape the content of a field to detect anly links to local SiteTree pages or files
 	 *
-	 * @todo - Replace image tracking with shortcodes
-	 *
 	 * @param string $fieldName The name of the field on {@link @owner} to scrape
 	 */
 	public function trackLinksInField($fieldName) {
@@ -108,6 +106,11 @@ class SiteTreeLinkTracking extends DataExtension {
 
 		// Highlight broken links in the content.
 		foreach ($links as $link) {
+			// Skip links without domelements
+			if(!isset($link['DOMReference'])) {
+				continue;
+			}
+
 			$classStr = trim($link['DOMReference']->getAttribute('class'));
 			if (!$classStr) {
 				$classes = array();
@@ -142,6 +145,7 @@ class SiteTreeLinkTracking extends DataExtension {
 					break;
 
 				case 'file':
+				case 'image':
 					if ($link['Broken']) {
 						$record->HasBrokenFile = true;
 					} else {
@@ -157,22 +161,6 @@ class SiteTreeLinkTracking extends DataExtension {
 			}
 		}
 
-		// Add file tracking for image references
-		if($images = $htmlValue->getElementsByTagName('img')) foreach($images as $img) {
-			// {@see HtmlEditorField} for data-fileid source
-			$fileID = $img->getAttribute('data-fileid');
-			if(!$fileID) {
-				continue;
-			}
-
-			// Assuming a local file is linked, check if it's valid
-			if($image = File::get()->byID($fileID)) {
-				$linkedFiles[] = $image->ID;
-			} else {
-				$record->HasBrokenFile = true;
-			}
-		}
-
 		// Update the "LinkTracking" many_many
 		if($record->ID && $record->manyManyComponent('LinkTracking') && ($tracker = $record->LinkTracking())) {
 			$tracker->removeByFilter(array(
@@ -274,15 +262,15 @@ class SiteTreeLinkTracking_Parser {
 
 			// Link to a page on this site.
 			$matches = array();
-			if(preg_match('/\[sitetree_link(?:\s*|%20|,)?id=([0-9]+)\](#(.*))?/i', $href, $matches)) {
+			if(preg_match('/\[sitetree_link(?:\s*|%20|,)?id=(?<id>[0-9]+)\](#(?<anchor>.*))?/i', $href, $matches)) {
-				$page = DataObject::get_by_id('SiteTree', $matches[1]);
+				$page = DataObject::get_by_id('SiteTree', $matches['id']);
 				$broken = false;
 
 				if (!$page) {
 					// Page doesn't exist.
 					$broken = true;
-				} else if (!empty($matches[3])) {
+				} else if (!empty($matches['anchor'])) {
-					$anchor = preg_quote($matches[3], '/');
+					$anchor = preg_quote($matches['anchor'], '/');
 
 					if (!preg_match("/(name|id)=\"{$anchor}\"/", $page->Content)) {
 						// Broken anchor on the target page.
@@ -292,8 +280,8 @@ class SiteTreeLinkTracking_Parser {
 
 				$results[] = array(
 					'Type' => 'sitetree',
-					'Target' => $matches[1],
+					'Target' => $matches['id'],
-					'Anchor' => empty($matches[3]) ? null : $matches[3],
+					'Anchor' => empty($matches['anchor']) ? null : $matches['anchor'],
 					'DOMReference' => $link,
 					'Broken' => $broken
 				);
@@ -303,13 +291,13 @@ class SiteTreeLinkTracking_Parser {
 
 			// Link to a file on this site.
 			$matches = array();
-			if(preg_match('/\[file_link(?:\s*|%20|,)?id=([0-9]+)\]/i', $href, $matches)) {
+			if(preg_match('/\[file_link(?:\s*|%20|,)?id=(?<id>[0-9]+)/i', $href, $matches)) {
 				$results[] = array(
 					'Type' => 'file',
-					'Target' => $matches[1],
+					'Target' => $matches['id'],
 					'Anchor' => null,
 					'DOMReference' => $link,
-					'Broken' => !DataObject::get_by_id('File', $matches[1])
+					'Broken' => !DataObject::get_by_id('File', $matches['id'])
 				);
 
 				continue;
@@ -331,6 +319,19 @@ class SiteTreeLinkTracking_Parser {
 
 		}
 
+		// Find all [image ] shortcodes (will be inline, not inside attributes)
+		$content = $htmlValue->getContent();
+		if(preg_match_all('/\[image([^\]]+)\bid=(["])?(?<id>\d+)\D/i', $content, $matches)) {
+			foreach($matches['id'] as $id) {
+				$results[] = array(
+					'Type' => 'image',
+					'Target' => (int)$id,
+					'Anchor' => null,
+					'DOMReference' => null,
+					'Broken' => !DataObject::get_by_id('Image', (int)$id)
+				);
+			}
+		}
 		return $results;
 	}
 

tests/model/FileLinkTrackingTest.php

@@ -28,7 +28,7 @@ class FileLinkTrackingTest extends SapphireTest {
 		$imageID = $this->idFromFixture('Image', 'file1');
 		$page = $this->objFromFixture('Page', 'page1');
 		$page->Content = sprintf(
-			'<p><img src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg" data-fileid="%d" /></p>',
+			'<p>[image src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg" id="%d"]</p>',
 			$imageID
 		);
 		$page->write();
@@ -39,20 +39,28 @@ class FileLinkTrackingTest extends SapphireTest {
 		parent::tearDown();
 	}
 
+	/**
+	 * Test uses global state through Versioned::set_reading_mode() since
+	 * the shortcode parser doesn't pass along the underlying DataObject
+	 * context, hence we can't call getSourceQueryParams().
+	 */
 	public function testFileRenameUpdatesDraftAndPublishedPages() {
 		$page = $this->objFromFixture('Page', 'page1');
 		$page->doPublish();
 
 		// Live and stage pages both have link to public file
+		Versioned::set_stage(Versioned::DRAFT);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
+		Versioned::set_stage(Versioned::LIVE);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree_Live\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
 
+		Versioned::set_stage(Versioned::DRAFT);
 		$file = $this->objFromFixture('Image', 'file1');
 		$file->Name = 'renamed-test-file.jpg';
 		$file->write();
@@ -60,38 +68,43 @@ class FileLinkTrackingTest extends SapphireTest {
 		// Staged record now points to secure URL of renamed file, live record remains unchanged
 		// Note that the "secure" url doesn't have the "FileLinkTrackingTest" component because
 		// the mocked test location disappears for secure files.
+		Versioned::set_stage(Versioned::DRAFT);
 		$this->assertContains(
 			'<img src="/assets/55b443b601/renamed-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
+		Versioned::set_stage(Versioned::LIVE);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree_Live\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
 
 		// Publishing the file should result in a direct public link (indicated by "FileLinkTrackingTest")
 		// Although the old live page will still point to the old record.
 		// @todo - Ensure shortcodes are used with all images to prevent live records having broken links
 		$file->doPublish();
+		Versioned::set_stage(Versioned::DRAFT);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/renamed-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
+		Versioned::set_stage(Versioned::LIVE);
 		$this->assertContains(
-			// Note: Broken link until shortcode-enabled
+			'<img src="/assets/FileLinkTrackingTest/55b443b601/renamed-test-file.jpg"',
-			'<img src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg"',
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree_Live\" WHERE \"ID\" = ?", array($page->ID))->value()
 		);
 
-		// Publishing the page after publishing the asset will resolve any link issues
+		// Publishing the page after publishing the asset should retain linking
 		$page->doPublish();
+		Versioned::set_stage(Versioned::DRAFT);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/renamed-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
+		Versioned::set_stage(Versioned::LIVE);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/renamed-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree_Live\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
 	}
 
@@ -112,20 +125,20 @@ class FileLinkTrackingTest extends SapphireTest {
 		$file->write();
 
 		// Verify that the draft virtual pages have the correct content
-		$svp = Versioned::get_by_stage('VirtualPage', Versioned::DRAFT)->byID($svp->ID);
+		Versioned::set_stage(Versioned::DRAFT);
 		$this->assertContains(
 			'<img src="/assets/55b443b601/renamed-test-file.jpg"',
-			$svp->Content
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
 
 		// Publishing both file and page will update the live record
 		$file->doPublish();
 		$page->doPublish();
 
-		$svp = Versioned::get_by_stage('VirtualPage', Versioned::LIVE)->byID($svp->ID);
+		Versioned::set_stage(Versioned::LIVE);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/renamed-test-file.jpg"',
-			$svp->Content
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
 	}
 
@@ -151,11 +164,15 @@ class FileLinkTrackingTest extends SapphireTest {
 	public function testTwoFileRenamesInARowWork() {
 		$page = $this->objFromFixture('Page', 'page1');
 		$this->assertTrue($page->doPublish());
+
+		Versioned::set_stage(Versioned::LIVE);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree_Live\" WHERE \"ID\" = ?", array($page->ID))->value());
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
+		);
 
 		// Rename the file twice
+		Versioned::set_stage(Versioned::DRAFT);
 		$file = $this->objFromFixture('Image', 'file1');
 		$file->Name = 'renamed-test-file.jpg';
 		$file->write();
@@ -168,16 +185,18 @@ class FileLinkTrackingTest extends SapphireTest {
 		$file->doPublish();
 
 		// Confirm that the correct image is shown in both the draft and live site
+		Versioned::set_stage(Versioned::DRAFT);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/renamed-test-file-second-time.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
 
 		// Publishing this record also updates live record
 		$page->doPublish();
+		Versioned::set_stage(Versioned::LIVE);
 		$this->assertContains(
 			'<img src="/assets/FileLinkTrackingTest/55b443b601/renamed-test-file-second-time.jpg"',
-			DB::prepared_query("SELECT \"Content\" FROM \"SiteTree_Live\" WHERE \"ID\" = ?", array($page->ID))->value()
+			Page::get()->byID($page->ID)->dbObject('Content')->forTemplate()
 		);
 	}
 }

tests/model/FileLinkTrackingTest.yml

@@ -9,4 +9,4 @@ Page:
   page1:
     Title: page1
     URLSegment: page1
-    Content: '<p><img src="/assets/FileLinkTrackingTest/55b443b601/testscript-test-file.jpg" /></p>'
+    # Content is set via test setup

tests/model/SiteTreeHTMLEditorFieldTest.php

@@ -124,7 +124,7 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 		$editor = new HtmlEditorField('Content');
 		$file = $this->objFromFixture('Image', 'example_image');
 
-		$editor->setValue(sprintf('<img src="%s" data-fileid="%d" />', $file->getURL(), $file->ID));
+		$editor->setValue(sprintf('[image src="%s" id="%d"]', $file->getURL(), $file->ID));
 		$editor->saveInto($sitetree);
 		$sitetree->write();
 		$this->assertEquals(

tests/model/SiteTreeLinkTrackingTest.php

@@ -62,8 +62,8 @@ class SiteTreeLinkTrackingTest extends SapphireTest {
 	}
 
 	public function testHasBrokenFile() {
-		$this->assertTrue($this->pageIsBrokenFile('<img src="someurl.jpg" data-fileid="99999999" />'));
+		$this->assertTrue($this->pageIsBrokenFile('[image src="someurl.jpg" id="99999999"]'));
-		$this->assertFalse($this->pageIsBrokenFile('<img src="someurl.jpg" />'));
+		$this->assertFalse($this->pageIsBrokenFile('[image src="someurl.jpg"]'));
 	}
 
 	protected function pageIsBrokenFile($content) {