Merge pull request #691 from nextcloud/bugfix/686/clean-followers

Cleaning followers on deleted accounts

lib/Command/CheckInstall.php

@@ -31,13 +31,13 @@ declare(strict_types=1);
 namespace OCA\Social\Command;
 
 
+use daita\MySmallPhpTools\Traits\TArrayTools;
 use Exception;
 use OC\Core\Command\Base;
 use OCA\Social\Service\CheckService;
 use OCA\Social\Service\MiscService;
 use OCA\Social\Service\PushService;
 use OCP\IUserManager;
-use OCP\Push\Exceptions\PushInstallException;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -46,6 +46,9 @@ use Symfony\Component\Console\Output\OutputInterface;
 class CheckInstall extends Base {
 
 
+	use TArrayTools;
+
+
 	/** @var IUserManager */
 	private $userManager;
 
@@ -86,7 +89,8 @@ class CheckInstall extends Base {
 		parent::configure();
 		$this->setName('social:check:install')
 			 ->addOption(
-				 'push', '', InputOption::VALUE_REQUIRED, 'a local account used to test integration to Nextcloud Push',
+				 'push', '', InputOption::VALUE_REQUIRED,
+				 'a local account used to test integration to Nextcloud Push',
 				 ''
 			 )
 			 ->setDescription('Check the integrity of the installation');
@@ -100,9 +104,14 @@ class CheckInstall extends Base {
 	 * @throws Exception
 	 */
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		$this->checkService->checkInstallationStatus();
+		$result = $this->checkService->checkInstallationStatus();
 
-		$this->checkPushApp($input, $output);
+		if ($this->checkPushApp($input, $output)) {
+			return;
+		}
+
+		$output->writeln('- ' . $this->getInt('invalidFollowers', $result, 0) . ' invalid followers removed');
+		$output->writeln('- ' . $this->getInt('invalidNotes', $result, 0) . ' invalid notes removed');
 	}
 
 
@@ -110,20 +119,25 @@ class CheckInstall extends Base {
 	 * @param InputInterface $input
 	 * @param OutputInterface $output
 	 *
+	 * @return bool
 	 * @throws Exception
 	 */
-	private function checkPushApp(InputInterface $input, OutputInterface $output) {
+	private function checkPushApp(InputInterface $input, OutputInterface $output): bool {
 		$userId = $input->getOption('push');
-		if ($userId !== '') {
+		if ($userId === '') {
-			$user = $this->userManager->get($userId);
+			return false;
-			if ($user === null) {
-				throw new Exception('unknown user');
-			}
-
-			$wrapper = $this->pushService->testOnAccount($userId);
-
-			$output->writeln(json_encode($wrapper, JSON_PRETTY_PRINT));
 		}
+
+		$user = $this->userManager->get($userId);
+		if ($user === null) {
+			throw new Exception('unknown user');
+		}
+
+		$wrapper = $this->pushService->testOnAccount($userId);
+
+		$output->writeln(json_encode($wrapper, JSON_PRETTY_PRINT));
+
+		return true;
 	}
 
 }

lib/Controller/LocalController.php

@@ -325,11 +325,11 @@ class LocalController extends Controller {
 	public function postUnlike(string $postId): DataResponse {
 		try {
 			$this->initViewer(true);
-			$announce = $this->likeService->delete($this->viewer, $postId, $token);
+			$like = $this->likeService->delete($this->viewer, $postId, $token);
 
 			return $this->success(
 				[
-					'like'  => $announce,
+					'like'  => $like,
 					'token' => $token
 				]
 			);
@@ -728,7 +728,7 @@ class LocalController extends Controller {
 	public function globalActorAvatar(string $id): Response {
 		try {
 			$actor = $this->cacheActorService->getFromId($id);
-			if ($actor->gotIcon()) {
+			if ($actor->hasIcon()) {
 				$avatar = $actor->getIcon();
 				$mime = '';
 				$document = $this->documentService->getFromCache($avatar->getId(), $mime);

lib/Controller/NavigationController.php

@@ -149,7 +149,7 @@ class NavigationController extends Controller {
 		try {
 			$data['serverData']['cloudAddress'] = $this->configService->getCloudUrl();
 		} catch (SocialAppConfigException $e) {
-			$this->checkService->checkInstallationStatus();
+			$this->checkService->checkInstallationStatus(true);
 			$cloudAddress = $this->setupCloudAddress();
 			if ($cloudAddress !== '') {
 				$data['serverData']['cloudAddress'] = $cloudAddress;

lib/Db/CacheActorsRequest.php

@@ -102,7 +102,7 @@ class CacheActorsRequest extends CacheActorsRequestBuilder {
 		} catch (Exception $e) {
 		}
 
-		if ($actor->gotIcon()) {
+		if ($actor->hasIcon()) {
 			$iconId = $actor->getIcon()
 							->getId();
 		} else {
@@ -160,7 +160,7 @@ class CacheActorsRequest extends CacheActorsRequestBuilder {
 		} catch (Exception $e) {
 		}
 
-		if ($actor->gotIcon()) {
+		if ($actor->hasIcon()) {
 			$iconId = $actor->getIcon()
 							->getId();
 		} else {
@@ -301,13 +301,12 @@ class CacheActorsRequest extends CacheActorsRequestBuilder {
 	 *
 	 * @param string $id
 	 */
-	public function deleteFromId(string $id) {
+	public function deleteCacheById(string $id) {
 		$qb = $this->getCacheActorsDeleteSql();
 		$this->limitToIdString($qb, $id);
 
 		$qb->execute();
 	}
 
-
 }
 

lib/Db/FollowsRequest.php

@@ -92,6 +92,23 @@ class FollowsRequest extends FollowsRequestBuilder {
 	}
 
 
+	/**
+	 * @return Follow[]
+	 */
+	public function getAll(): array {
+		$qb = $this->getFollowsSelectSql();
+
+		$follows = [];
+		$cursor = $qb->execute();
+		while ($data = $cursor->fetch()) {
+			$follows[] = $this->parseFollowsSelectSql($data);
+		}
+		$cursor->closeCursor();
+
+		return $follows;
+	}
+
+
 	/**
 	 * @param string $actorId
 	 * @param string $remoteActorId
@@ -267,7 +284,6 @@ class FollowsRequest extends FollowsRequestBuilder {
 		$qb->execute();
 	}
 
-
 	/**
 	 * @param Follow $follow
 	 */
@@ -279,6 +295,30 @@ class FollowsRequest extends FollowsRequestBuilder {
 		$qb->execute();
 	}
 
+	/**
+	 * @param string $actorId
+	 */
+	public function deleteRelatedId(string $actorId) {
+		$qb = $this->getFollowsDeleteSql();
+		$this->limitToActorId($qb, $actorId);
+
+		$qb->execute();
+
+		$qb = $this->getFollowsDeleteSql();
+		$this->limitToObjectId($qb, $actorId);
+
+		$qb->execute();
+	}
+
+	/**
+	 * @param string $id
+	 */
+	public function deleteById(string $id) {
+		$qb = $this->getFollowsDeleteSql();
+		$this->limitToIdString($qb, $id);
+
+		$qb->execute();
+	}
 
 }
 

lib/Db/StreamRequest.php

@@ -195,6 +195,32 @@ class StreamRequest extends StreamRequestBuilder {
 	}
 
 
+	/**
+	 * @param string $type
+	 *
+	 * @return Stream[]
+	 */
+	public function getAll(string $type = ''): array {
+		$qb = $this->getStreamSelectSql();
+
+		if ($type !== '') {
+			$this->limitToType($qb, $type);
+		}
+
+		$streams = [];
+		$cursor = $qb->execute();
+		while ($data = $cursor->fetch()) {
+			try {
+				$streams[] = $this->parseStreamSelectSql($data);
+			} catch (Exception $e) {
+			}
+		}
+		$cursor->closeCursor();
+
+		return $streams;
+	}
+
+
 	/**
 	 * @param string $id
 	 * @param bool $asViewer
@@ -614,10 +640,10 @@ class StreamRequest extends StreamRequestBuilder {
 	 * @param string $id
 	 * @param string $type
 	 */
-	public function deleteStreamById(string $id, string $type = '') {
+	public function deleteById(string $id, string $type = '') {
 		$qb = $this->getStreamDeleteSql();
-
 		$this->limitToIdString($qb, $id);
+
 		if ($type !== '') {
 			$this->limitToType($qb, $type);
 		}

lib/Interfaces/Activity/DeleteInterface.php

@@ -63,26 +63,19 @@ class DeleteInterface implements IActivityPubInterface {
 	 */
 	public function processIncomingRequest(ACore $item) {
 		$item->checkOrigin($item->getId());
+		$item->checkOrigin($item->getObjectId());
 
 		if (!$item->hasObject()) {
+			$types = ['Note', 'Person'];
+			foreach ($types as $type) {
+				try {
+					$interface = AP::$activityPub->getInterfaceFromType($type);
+					$object = $interface->getItemById($item->getObjectId());
+					$interface->delete($object);
 
-			if ($item->getObjectId() !== '') {
+					return;
-				$item->checkOrigin($item->getObjectId());
+				} catch (ItemNotFoundException $e) {
-
+				} catch (ItemUnknownException $e) {
-				// TODO: migrate to activity() !!
-				$types = ['Note', 'Person'];
-				foreach ($types as $type) {
-					try {
-						$item->checkOrigin($item->getObjectId());
-						$interface = AP::$activityPub->getInterfaceFromType($type);
-						$object = $interface->getItemById($item->getObjectId());
-						$interface->delete($object);
-
-						return;
-					} catch (InvalidOriginException $e) {
-					} catch (ItemNotFoundException $e) {
-					} catch (ItemUnknownException $e) {
-					}
 				}
 			}
 
@@ -91,11 +84,8 @@ class DeleteInterface implements IActivityPubInterface {
 
 		$object = $item->getObject();
 		try {
-			$item->checkOrigin($object->getId());
+			$interface = AP::$activityPub->getInterfaceForItem($object);
-			// FIXME: needed ? better use activity()
+			$interface->activity($item, $object);
-//			$interface = AP::$activityPub->getInterfaceForItem($object);
-//			$interface->delete($object);
-		} catch (InvalidOriginException $e) {
 		} catch (ItemUnknownException $e) {
 		}
 	}

lib/Interfaces/Actor/PersonInterface.php

@@ -33,6 +33,7 @@ namespace OCA\Social\Interfaces\Actor;
 
 use daita\MySmallPhpTools\Traits\TArrayTools;
 use OCA\Social\Db\CacheActorsRequest;
+use OCA\Social\Db\FollowsRequest;
 use OCA\Social\Db\StreamRequest;
 use OCA\Social\Exceptions\CacheActorDoesNotExistException;
 use OCA\Social\Exceptions\InvalidOriginException;
@@ -63,6 +64,9 @@ class PersonInterface implements IActivityPubInterface {
 	/** @var StreamRequest */
 	private $streamRequest;
 
+	/** @var FollowsRequest */
+	private $followsRequest;
+
 	/** @var ActorService */
 	private $actorService;
 
@@ -78,16 +82,19 @@ class PersonInterface implements IActivityPubInterface {
 	 *
 	 * @param CacheActorsRequest $cacheActorsRequest
 	 * @param StreamRequest $streamRequest
+	 * @param FollowsRequest $followsRequest
 	 * @param ActorService $actorService
 	 * @param ConfigService $configService
 	 * @param MiscService $miscService
 	 */
 	public function __construct(
 		CacheActorsRequest $cacheActorsRequest, StreamRequest $streamRequest,
-		ActorService $actorService, ConfigService $configService, MiscService $miscService
+		FollowsRequest $followsRequest, ActorService $actorService, ConfigService $configService,
+		MiscService $miscService
 	) {
 		$this->cacheActorsRequest = $cacheActorsRequest;
 		$this->streamRequest = $streamRequest;
+		$this->followsRequest = $followsRequest;
 		$this->actorService = $actorService;
 		$this->configService = $configService;
 		$this->miscService = $miscService;
@@ -178,8 +185,9 @@ class PersonInterface implements IActivityPubInterface {
 	 */
 	public function delete(ACore $item) {
 		/** @var Person $item */
-		$this->cacheActorsRequest->deleteFromId($item->getId());
+		$this->cacheActorsRequest->deleteCacheById($item->getId());
 		$this->streamRequest->deleteByAuthor($item->getId());
+		$this->followsRequest->deleteRelatedId($item->getId());
 	}
 
 

lib/Interfaces/Internal/SocialAppNotificationInterface.php

@@ -157,7 +157,7 @@ class SocialAppNotificationInterface implements IActivityPubInterface {
 	 */
 	public function delete(ACore $item) {
 		/** @var Stream $item */
-		$this->streamRequest->deleteStreamById($item->getId(), SocialAppNotification::TYPE);
+		$this->streamRequest->deleteById($item->getId(), SocialAppNotification::TYPE);
 	}
 
 

lib/Interfaces/Object/AnnounceInterface.php

@@ -277,7 +277,7 @@ class AnnounceInterface implements IActivityPubInterface {
 			$knownItem->removeCc($actor->getFollowers());
 
 			if (empty($knownItem->getCcArray())) {
-				$this->streamRequest->deleteStreamById($knownItem->getId(), Announce::TYPE);
+				$this->streamRequest->deleteById($knownItem->getId(), Announce::TYPE);
 			} else {
 				$this->streamRequest->update($knownItem);
 			}

lib/Interfaces/Object/NoteInterface.php

@@ -38,6 +38,7 @@ use OCA\Social\Exceptions\StreamNotFoundException;
 use OCA\Social\Interfaces\IActivityPubInterface;
 use OCA\Social\Model\ActivityPub\ACore;
 use OCA\Social\Model\ActivityPub\Activity\Create;
+use OCA\Social\Model\ActivityPub\Activity\Delete;
 use OCA\Social\Model\ActivityPub\Object\Note;
 use OCA\Social\Service\ConfigService;
 use OCA\Social\Service\CurlService;
@@ -140,6 +141,11 @@ class NoteInterface implements IActivityPubInterface {
 			$item->setActivityId($activity->getId());
 			$this->save($item);
 		}
+
+		if ($activity->getType() === Delete::TYPE) {
+			$activity->checkOrigin($item->getId());
+			$this->delete($item);
+		}
 	}
 
 
@@ -167,14 +173,10 @@ class NoteInterface implements IActivityPubInterface {
 
 	/**
 	 * @param ACore $item
-	 *
-	 * @throws InvalidOriginException
 	 */
 	public function delete(ACore $item) {
-		$item->checkOrigin(($item->getId()));
-
 		/** @var Note $item */
-		$this->streamRequest->deleteStreamById($item->getId(), Note::TYPE);
+		$this->streamRequest->deleteById($item->getId(), Note::TYPE);
 	}
 
 

lib/Migration/CheckInstallation.php

@@ -73,7 +73,7 @@ class CheckInstallation implements IRepairStep {
 	 * @param IOutput $output
 	 */
 	public function run(IOutput $output) {
-		$this->checkService->checkInstallationStatus();
+		$this->checkService->checkInstallationStatus(true);
 	}
 
 

lib/Model/ActivityPub/ACore.php

@@ -171,6 +171,18 @@ class ACore extends Item implements JsonSerializable {
 		return $this;
 	}
 
+	/**
+	 * @return string
+	 */
+	public function getObjectId(): string {
+		if ($this->hasObject()) {
+			return $this->getObject()
+						->getId();
+		}
+
+		return parent::getObjectId();
+	}
+
 
 	/**
 	 * @param bool $filter - will remove general url like Public
@@ -191,7 +203,7 @@ class ACore extends Item implements JsonSerializable {
 	/**
 	 * @return bool
 	 */
-	public function gotIcon(): bool {
+	public function hasIcon(): bool {
 		if ($this->icon === null) {
 			return false;
 		}
@@ -306,7 +318,7 @@ class ACore extends Item implements JsonSerializable {
 		$origin = $this->getRoot()
 					   ->getOrigin();
 
-		if ($origin === $host) {
+		if ($id !== '' && $origin === $host && $host !== '') {
 			return;
 		}
 
@@ -684,7 +696,7 @@ class ACore extends Item implements JsonSerializable {
 		}
 
 		// TODO - moving the $this->icon to Model/Person ?
-		if ($this->gotIcon()) {
+		if ($this->hasIcon()) {
 			$this->addEntryItem('icon', $this->getIcon());
 		}
 

lib/Service/ActorService.php

@@ -131,7 +131,7 @@ class ActorService {
 	 * @param Person $actor
 	 */
 	private function cacheDocumentIfNeeded(Person $actor) {
-		if ($actor->gotIcon()) {
+		if ($actor->hasIcon()) {
 			$icon = $actor->getIcon();
 			try {
 				$cache = $this->cacheDocumentsRequest->getByUrl($icon->getUrl());

lib/Service/CheckService.php

@@ -28,8 +28,12 @@ use daita\MySmallPhpTools\Traits\TArrayTools;
 use daita\MySmallPhpTools\Traits\TStringTools;
 use Exception;
 use GuzzleHttp\Exception\ClientException;
+use OCA\Social\Db\CacheActorsRequest;
 use OCA\Social\Db\FollowsRequest;
+use OCA\Social\Db\StreamRequest;
+use OCA\Social\Exceptions\CacheActorDoesNotExistException;
 use OCA\Social\Model\ActivityPub\Object\Follow;
+use OCA\Social\Model\ActivityPub\Object\Note;
 use OCP\AppFramework\Http;
 use OCP\Http\Client\IClientService;
 use OCP\ICache;
@@ -68,11 +72,20 @@ class CheckService {
 	/** @var IURLGenerator */
 	private $urlGenerator;
 
+	/** @var FollowsRequest */
+	private $followRequest;
+
+	/** @var CacheActorsRequest */
+	private $cacheActorsRequest;
+
+	/** @var StreamRequest */
+	private $streamRequest;
+
 	/** @var ConfigService */
 	private $configService;
 
-	/** @var FollowsRequest */
+	/** @var MiscService */
-	private $followRequest;
+	private $miscService;
 
 
 	/**
@@ -84,11 +97,17 @@ class CheckService {
 	 * @param IRequest $request
 	 * @param IURLGenerator $urlGenerator
 	 * @param FollowsRequest $followRequest
+	 * @param CacheActorsRequest $cacheActorsRequest
+	 * @param StreamRequest $streamRequest
 	 * @param ConfigService $configService
+	 * @param MiscService $miscService
 	 */
 	public function __construct(
 		ICache $cache, IConfig $config, IClientService $clientService, IRequest $request,
-		IURLGenerator $urlGenerator, FollowsRequest $followRequest, ConfigService $configService
+		IURLGenerator $urlGenerator, FollowsRequest $followRequest,
+		CacheActorsRequest $cacheActorsRequest, StreamRequest $streamRequest,
+		ConfigService $configService,
+		MiscService $miscService
 	) {
 		$this->cache = $cache;
 		$this->config = $config;
@@ -96,7 +115,10 @@ class CheckService {
 		$this->request = $request;
 		$this->urlGenerator = $urlGenerator;
 		$this->followRequest = $followRequest;
+		$this->cacheActorsRequest = $cacheActorsRequest;
+		$this->streamRequest = $streamRequest;
 		$this->configService = $configService;
+		$this->miscService = $miscService;
 	}
 
 
@@ -151,12 +173,24 @@ class CheckService {
 
 
 	/**
+	 * @param bool $light
 	 *
+	 * @return array
 	 */
-	public function checkInstallationStatus() {
+	public function checkInstallationStatus(bool $light = false): array {
 		$this->configService->setCoreValue('public_webfinger', 'social/lib/webfinger.php');
 		$this->configService->setCoreValue('public_host-meta', 'social/lib/hostmeta.php');
+
+		if (!$light) {
+			$result = [
+				'invalidFollows' => $this->removeInvalidFollows(),
+				'invalidNotes'   => $this->removeInvalidNotes()
+			];
+		}
+
 		$this->checkStatusTableFollows();
+
+		return $result;
 	}
 
 
@@ -179,6 +213,50 @@ class CheckService {
 	}
 
 
+	/**
+	 * @return int
+	 */
+	public function removeInvalidFollows(): int {
+		$count = 0;
+		$follows = $this->followRequest->getAll();
+		foreach ($follows as $follow) {
+			try {
+				$this->cacheActorsRequest->getFromId($follow->getActorId());
+				$this->cacheActorsRequest->getFromId($follow->getObjectId());
+			} catch (CacheActorDoesNotExistException $e) {
+				$this->followRequest->deleteById($follow->getId());
+				$count++;
+			}
+		}
+
+		$this->miscService->log('removeInvalidFollows removed ' . $count . ' entries', 1);
+
+		return $count;
+	}
+
+
+	/**
+	 * @return int
+	 */
+	public function removeInvalidNotes(): int {
+		$count = 0;
+		$streams = $this->streamRequest->getAll(Note::TYPE);
+		foreach ($streams as $stream) {
+			try {
+				// Check if it's enough for Note, Announce, ...
+				$this->cacheActorsRequest->getFromId($stream->getAttributedTo());
+			} catch (CacheActorDoesNotExistException $e) {
+				$this->streamRequest->deleteById($stream->getId(), Note::TYPE);
+				$count++;
+			}
+		}
+
+		$this->miscService->log('removeInvalidNotes removed ' . $count . ' entries', 1);
+
+		return $count;
+	}
+
+
 	/**
 	 * @param string $base
 	 *

lib/Service/StreamService.php

@@ -363,7 +363,7 @@ class StreamService {
 
 		$item->setActorId($item->getAttributedTo());
 		$this->activityService->deleteActivity($item);
-		$this->streamRequest->deleteStreamById($item->getId(), $type);
+		$this->streamRequest->deleteById($item->getId(), $type);
 	}