Merge pull request #10182 from annando/avoid-duplicates

Avoid duplicated links in shared posts

src/Content/Text/BBCode.php

@@ -61,7 +61,8 @@ class BBCode
 	const BACKLINK = 8;
 	const ACTIVITYPUB = 9;
 
-	const ANCHOR = '<br class="anchor">';
+	const TOP_ANCHOR = '<br class="top-anchor">';
+	const BOTTOM_ANCHOR = '<br class="button-anchor">';
 	/**
 	 * Fetches attachment data that were generated the old way
 	 *
@@ -1089,7 +1090,7 @@ class BBCode
 					'$guid'         => $attributes['guid'],
 					'$network_name' => ContactSelector::networkToName($network, $attributes['profile']),
 					'$network_icon' => ContactSelector::networkToIcon($network, $attributes['profile']),
-					'$content'      => self::setMentions(trim($content), 0, $network) . self::ANCHOR,
+					'$content'      => self::TOP_ANCHOR . self::setMentions(trim($content), 0, $network) . self::BOTTOM_ANCHOR,
 				]);
 				break;
 		}

src/Model/Item.php

@@ -184,6 +184,7 @@ class Item
 				$content_fields = ['raw-body' => trim($fields['raw-body'] ?? $fields['body'])];
 
 				// Remove all media attachments from the body and store them in the post-media table
+				// @todo On shared postings (Diaspora style and commented reshare) don't fetch content from the shared part
 				$content_fields['raw-body'] = Post\Media::insertFromBody($item['uri-id'], $content_fields['raw-body']);
 				$content_fields['raw-body'] = self::setHashtags($content_fields['raw-body']);
 			}
@@ -2639,10 +2640,10 @@ class Item
 			unset($hook_data);
 		}
 
-		$orig_body = $item['body'];
+		$body = $item['body'] ?? '';
 		$item['body'] = preg_replace("/\s*\[attachment .*?\].*?\[\/attachment\]\s*/ism", '', $item['body']);
 		self::putInCache($item);
-		$item['body'] = $orig_body;
+		$item['body'] = $body;
 		$s = $item["rendered-html"];
 
 		$hook_data = [
@@ -2666,19 +2667,23 @@ class Item
 		if (!empty($shared['guid'])) {
 			$shared_item = Post::selectFirst(['uri-id', 'plink'], ['guid' => $shared['guid']]);
 			$shared_uri_id = $shared_item['uri-id'] ?? 0;
-			$shared_plink = $shared_item['plink'] ?? '';
+			$shared_links = [strtolower($shared_item['plink'] ?? '')];
 			$attachments = Post\Media::splitAttachments($shared_uri_id, $shared['guid']);
 			$s = self::addVisualAttachments($attachments, $item, $s, true);
-			$s = self::addLinkAttachment($attachments, $item, $s, true, '');
+			$s = self::addLinkAttachment($attachments, $body, $s, true, []);
 			$s = self::addNonVisualAttachments($attachments, $item, $s, true);
+			$shared_links = array_merge($shared_links, array_column($attachments['visual'], 'url'));
+			$shared_links = array_merge($shared_links, array_column($attachments['link'], 'url'));
+			$shared_links = array_merge($shared_links, array_column($attachments['additional'], 'url'));
+			$body = preg_replace("/\s*\[share .*?\].*?\[\/share\]\s*/ism", '', $body);
 		} else {
 			$shared_uri_id = 0;
-			$shared_plink = '';
+			$shared_links = [];
 		}
 
-		$attachments = Post\Media::splitAttachments($item['uri-id'], $item['guid']);
+		$attachments = Post\Media::splitAttachments($item['uri-id'], $item['guid'], $shared_links);
 		$s = self::addVisualAttachments($attachments, $item, $s, false);
-		$s = self::addLinkAttachment($attachments, $item, $s, false, $shared_plink);
+		$s = self::addLinkAttachment($attachments, $body, $s, false, $shared_links);
 		$s = self::addNonVisualAttachments($attachments, $item, $s, false);
 
 		// Map.
@@ -2738,6 +2743,7 @@ class Item
 		$leading = '';
 		$trailing = '';
 
+		// @todo In the future we should make a single for the template engine with all media in it. This allows more flexibilty.
 		foreach ($attachments['visual'] as $attachment) {
 			if (self::containsLink($item['body'], $attachment['url'])) {
 				continue;
@@ -2794,13 +2800,18 @@ class Item
 						'attachment'   => $attachment,
 					],
 				]);
-				$trailing .= $media;
+				// On Diaspora posts the attached pictures are leading
+				if ($item['network'] == Protocol::DIASPORA) {
+					$leading .= $media;
+				} else {
+					$trailing .= $media;
+				}
 			}
 		}
 
 		if ($shared) {
-			$content = str_replace(BBCode::ANCHOR, '<div class="body-attach">' . $leading . '<div class="clear"></div></div>' . BBCode::ANCHOR, $content);
+			$content = str_replace(BBCode::TOP_ANCHOR, '<div class="body-attach">' . $leading . '<div class="clear"></div></div>' . BBCode::TOP_ANCHOR, $content);
-			$content = str_replace(BBCode::ANCHOR, BBCode::ANCHOR . '<div class="body-attach">' . $trailing . '<div class="clear"></div></div>', $content);
+			$content = str_replace(BBCode::BOTTOM_ANCHOR, '<div class="body-attach">' . $trailing . '<div class="clear"></div></div>' . BBCode::BOTTOM_ANCHOR, $content);
 		} else {
 			if ($leading != '') {
 				$content = '<div class="body-attach">' . $leading . '<div class="clear"></div></div>' . $content;
@@ -2819,12 +2830,13 @@ class Item
 	 * Add link attachment to the content
 	 *
 	 * @param array  $attachments
-	 * @param array  $item
+	 * @param string $body
 	 * @param string $content
 	 * @param bool   $shared
+	 * @param array  $ignore_links A list of URLs to ignore
 	 * @return string modified content
 	 */
-	private static function addLinkAttachment(array $attachments, array $item, string $content, bool $shared, string $ignore_link)
+	private static function addLinkAttachment(array $attachments, string $body, string $content, bool $shared, array $ignore_links)
 	{
 		$stamp1 = microtime(true);
 		// @ToDo Check only for audio and video
@@ -2832,14 +2844,19 @@ class Item
 
 		if (!empty($attachments['link'])) {
 			foreach ($attachments['link'] as $link) {
-				if (!Strings::compareLink($link['url'], $ignore_link)) {
+				$found = false;
+				foreach ($ignore_links as $ignore_link) {
+					if (Strings::compareLink($link['url'], $ignore_link)) {
+						$found = true;
+					}
+				}
+				if (!$found) {
 					$attachment = $link;
 				}
 			}
 		}
 
 		if (!empty($attachment)) {
-			$footer = '';
 			$data = [
 				'after' => '',
 				'author_name' => $attachment['author-name'] ?? '',
@@ -2861,17 +2878,16 @@ class Item
 					$data['preview'] = $attachment['preview'] ?? '';
 				}
 			}
-		} elseif (preg_match("/.*(\[attachment.*?\].*?\[\/attachment\]).*/ism", $item['body'], $match)) {
+		} elseif (preg_match("/.*(\[attachment.*?\].*?\[\/attachment\]).*/ism", $body, $match)) {
-			$footer = $match[1];
+			$data = BBCode::getAttachmentData($match[1]);
-			$data = [];
 		}
 		DI::profiler()->saveTimestamp($stamp1, 'rendering');
 
-		if (!empty($footer) || !empty($data)) {
+		if (isset($data['url']) && !in_array($data['url'], $ignore_links)) {
 			// @todo Use a template
-			$rendered = BBCode::convertAttachment($footer, BBCode::INTERNAL, false, $data);
+			$rendered = BBCode::convertAttachment('', BBCode::INTERNAL, false, $data);
 			if ($shared) {
-				return str_replace(BBCode::ANCHOR, BBCode::ANCHOR . $rendered, $content);
+				return str_replace(BBCode::BOTTOM_ANCHOR, BBCode::BOTTOM_ANCHOR . $rendered, $content);
 			} else {
 				return $content . $rendered;
 			}

src/Model/Post/Media.php

@@ -28,8 +28,11 @@ use Friendica\Core\System;
 use Friendica\Database\Database;
 use Friendica\Database\DBA;
 use Friendica\DI;
+use Friendica\Model\Item;
+use Friendica\Model\Post;
 use Friendica\Util\Images;
 use Friendica\Util\ParseUrl;
+use Friendica\Util\Strings;
 
 /**
  * Class Media
@@ -447,11 +450,12 @@ class Media
 	/**
 	 * Split the attachment media in the three segments "visual", "link" and "additional"
 	 * 
-	 * @param int $uri_id 
+	 * @param int    $uri_id 
 	 * @param string $guid
+	 * @param array  $links ist of links that shouldn't be added 
 	 * @return array attachments
 	 */
-	public static function splitAttachments(int $uri_id, string $guid = '')
+	public static function splitAttachments(int $uri_id, string $guid = '', array $links = [])
 	{
 		$attachments = ['visual' => [], 'link' => [], 'additional' => []];
 
@@ -464,6 +468,12 @@ class Media
 		$selected = '';
 
 		foreach ($media as $medium) {
+			foreach ($links as $link) {
+				if (Strings::compareLink($link, $medium['url'])) {
+					continue 2;
+				}
+			}
+
 			$type = explode('/', current(explode(';', $medium['mimetype'])));
 			if (count($type) < 2) {
 				Logger::info('Unknown MimeType', ['type' => $type, 'media' => $medium]);
@@ -511,4 +521,47 @@ class Media
 		}
 		return $attachments;
 	}
+
+	/**
+	 * Add media attachments to the body
+	 *
+	 * @param int $uriid
+	 * @param string $body
+	 * @return string body
+	 */
+	public static function addAttachmentsToBody(int $uriid, string $body = '')
+	{
+		if (empty($body)) {
+			$item = Post::selectFirst(['body'], ['uri-id' => $uriid]);
+			if (!DBA::isResult($item)) {
+				return '';
+			}
+			$body = $item['body'];
+		}
+		$body = preg_replace("/\s*\[attachment .*?\].*?\[\/attachment\]\s*/ism", '', $body);
+
+		foreach (self::getByURIId($uriid, [self::IMAGE, self::AUDIO, self::VIDEO]) as $media) {
+			if (Item::containsLink($body, $media['url'])) {
+				continue;
+			}
+
+			if ($media['type'] == self::IMAGE) {
+				if (!empty($media['description'])) {
+					$body .= "\n[img=" . $media['url'] . ']' . $media['description'] .'[/img]';
+				} else {
+					$body .= "\n[img]" . $media['url'] .'[/img]';
+				}
+			} elseif ($media['type'] == self::AUDIO) {
+				$body .= "\n[audio]" . $media['url'] . "[/audio]\n";
+			} elseif ($media['type'] == self::VIDEO) {
+				$body .= "\n[video]" . $media['url'] . "[/video]\n";
+			}
+		}
+
+		if (preg_match("/.*(\[attachment.*?\].*?\[\/attachment\]).*/ism", $item['body'], $match)) {
+			$body .= "\n" . $match[1];
+		}
+
+		return $body;
+	}
 }

src/Protocol/ActivityPub/Transmitter.php

@@ -1291,22 +1291,35 @@ class Transmitter
 			$attachments[] = $attachment;
 		}
 		*/
-		foreach (Post\Media::getByURIId($item['uri-id'], [Post\Media::DOCUMENT, Post\Media::TORRENT, Post\Media::UNKNOWN]) as $attachment) {
+		$uriids = [$item['uri-id']];
-			$attachments[] = ['type' => 'Document',
+		$shared = BBCode::fetchShareAttributes($item['body']);
-				'mediaType' => $attachment['mimetype'],
+		if (!empty($shared['guid'])) {
-				'url' => $attachment['url'],
+			$shared_item = Post::selectFirst(['uri-id'], ['guid' => $shared['guid']]);
-				'name' => $attachment['description']];
+			if (!empty($shared_item['uri-id'])) {
+				$uriids[] = $shared_item['uri-id'];
+			}
+		}
+
+		foreach ($uriids as $uriid) {
+			foreach (Post\Media::getByURIId($uriid, [Post\Media::DOCUMENT, Post\Media::TORRENT, Post\Media::UNKNOWN]) as $attachment) {
+				$attachments[] = ['type' => 'Document',
+					'mediaType' => $attachment['mimetype'],
+					'url' => $attachment['url'],
+					'name' => $attachment['description']];
+			}
 		}
 
 		if ($type != 'Note') {
 			return $attachments;
 		}
 
-		foreach (Post\Media::getByURIId($item['uri-id'], [Post\Media::AUDIO, Post\Media::IMAGE, Post\Media::VIDEO]) as $attachment) {
+		foreach ($uriids as $uriid) {
-			$attachments[] = ['type' => 'Document',
+			foreach (Post\Media::getByURIId($uriid, [Post\Media::AUDIO, Post\Media::IMAGE, Post\Media::VIDEO]) as $attachment) {
-				'mediaType' => $attachment['mimetype'],
+				$attachments[] = ['type' => 'Document',
-				'url' => $attachment['url'],
+					'mediaType' => $attachment['mimetype'],
-				'name' => $attachment['description']];
+					'url' => $attachment['url'],
+					'name' => $attachment['description']];
+			}
 		}
 
 		return $attachments;

src/Protocol/DFRN.php

@@ -899,10 +899,10 @@ class DFRN
 			$entry->setAttribute("xmlns:statusnet", ActivityNamespace::STATUSNET);
 		}
 
+		$body = Post\Media::addAttachmentsToBody($item['uri-id'], $item['body']);
+
 		if ($item['private'] == Item::PRIVATE) {
-			$body = Item::fixPrivatePhotos($item['body'], $owner['uid'], $item, $cid);
+			$body = Item::fixPrivatePhotos($body, $owner['uid'], $item, $cid);
-		} else {
-			$body = $item['body'];
 		}
 
 		// Remove the abstract element. It is only locally important.

src/Protocol/Diaspora.php

@@ -3366,36 +3366,6 @@ class Diaspora
 		return $eventdata;
 	}
 
-	/**
-	 * Add media attachments to the body
-	 *
-	 * @param array $item
-	 * @return string body
-	 */
-	private static function addAttachments(array $item)
-	{
-		$body = $item['body'];
-
-		foreach (Post\Media::getByURIId($item['uri-id'], [Post\Media::IMAGE, Post\Media::AUDIO, Post\Media::VIDEO]) as $media) {
-			if (Item::containsLink($item['body'], $media['url'])) {
-				continue;
-			}
-
-			if ($media['type'] == Post\Media::IMAGE) {
-				if (!empty($media['description'])) {
-					$body .= "\n[img=" . $media['url'] . ']' . $media['description'] .'[/img]';
-				} else {
-					$body .= "\n[img]" . $media['url'] .'[/img]';
-				}
-			} elseif ($media['type'] == Post\Media::AUDIO) {
-				$body .= "\n[audio]" . $media['url'] . "[/audio]\n";
-			} elseif ($media['type'] == Post\Media::VIDEO) {
-				$body .= "\n[video]" . $media['url'] . "[/video]\n";
-			}
-		}
-		return $body;
-	}
-
 	/**
 	 * Create a post (status message or reshare)
 	 *
@@ -3436,7 +3406,7 @@ class Diaspora
 			$type = "reshare";
 		} else {
 			$title = $item["title"];
-			$body = self::addAttachments($item);
+			$body = Post\Media::addAttachmentsToBody($item['uri-id'], $item['body']);
 
 			// Fetch the title from an attached link - if there is one
 			if (empty($item["title"]) && DI::pConfig()->get($owner['uid'], 'system', 'attach_link_title')) {
@@ -3650,7 +3620,7 @@ class Diaspora
 			$thread_parent_item = Post::selectFirst(['guid', 'author-id', 'author-link', 'gravity'], ['uri' => $item['thr-parent'], 'uid' => $item['uid']]);
 		}
 
-		$body = self::addAttachments($item);
+		$body = Post\Media::addAttachmentsToBody($item['uri-id'], $item['body']);
 
 		// The replied to autor mention is prepended for clarity if:
 		// - Item replied isn't yours

src/Protocol/OStatus.php

@@ -1885,7 +1885,8 @@ class OStatus
 		XML::addElement($doc, $entry, "id", $item["uri"]);
 		XML::addElement($doc, $entry, "title", html_entity_decode($title, ENT_QUOTES, 'UTF-8'));
 
-		$body = self::formatPicturePost($item['body']);
+		$body = Post\Media::addAttachmentsToBody($item['uri-id'], $item['body']);
+		$body = self::formatPicturePost($body);
 
 		if (!empty($item['title'])) {
 			$body = "[b]".$item['title']."[/b]\n\n".$body;