refactor(integration): Refactor and fix federation integration tests

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>

apps/federatedfilesharing/lib/FederatedShareProvider.php

@@ -63,24 +63,19 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl
 	) {
 	}
 
-	/**
-	 * Return the identifier of this provider.
-	 *
-	 * @return string Containing only [a-zA-Z0-9]
-	 */
-	public function identifier() {
+	#[Override]
+	public function identifier(): string {
 		return 'ocFederatedSharing';
 	}
 
 	/**
 	 * Share a path
 	 *
-	 * @param IShare $share
-	 * @return IShare The share object
 	 * @throws ShareNotFound
 	 * @throws \Exception
 	 */
-	public function create(IShare $share) {
+	#[Override]
+	public function create(IShare $share): IShare {
 		$shareWith = $share->getSharedWith();
 		$itemSource = $share->getNodeId();
 		$itemType = $share->getNodeType();
@@ -141,7 +136,7 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl
 		if ($remoteShare) {
 			try {
 				$ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']);
-				$shareId = (string)$this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
+				$shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
 				$share->setId($shareId);
 				[$token, $remoteId] = $this->askOwnerToReShare($shareWith, $share, $shareId);
 				// remote share was create successfully if we get a valid token as return
@@ -176,7 +171,7 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl
 	 */
 	protected function createFederatedShare(IShare $share): string {
 		$token = $this->tokenHandler->generateToken();
-		$shareId = (string)$this->addShareToDB(
+		$shareId = $this->addShareToDB(
 			$share->getNodeId(),
 			$share->getNodeType(),
 			$share->getSharedWith(),
@@ -292,9 +287,8 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl
 	 * @param string $token
 	 * @param int $shareType
 	 * @param \DateTime $expirationDate
-	 * @return int
 	 */
-	private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate) {
+	private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate): string {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->insert('share')
 			->setValue('share_type', $qb->createNamedParameter($shareType))
@@ -316,7 +310,7 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl
 		$qb->setValue('file_target', $qb->createNamedParameter(''));
 
 		$qb->executeStatement();
-		return $qb->getLastInsertId();
+		return (string)$qb->getLastInsertId();
 	}
 
 	/**

apps/files_external/tests/Config/UserPlaceholderHandlerTest.php

@@ -14,9 +14,11 @@ use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\MockObject\MockObject;
+use Test\TestCase;
 
-class UserPlaceholderHandlerTest extends \Test\TestCase {
+class UserPlaceholderHandlerTest extends TestCase {
 	protected IUser&MockObject $user;
 	protected IUserSession&MockObject $session;
 	protected IManager&MockObject $shareManager;
@@ -34,6 +36,9 @@ class UserPlaceholderHandlerTest extends \Test\TestCase {
 		$this->session = $this->createMock(IUserSession::class);
 		$this->shareManager = $this->createMock(IManager::class);
 		$this->request = $this->createMock(IRequest::class);
+		$this->request->method('getParam')
+			->with('token')
+			->willReturn('foo');
 		$this->userManager = $this->createMock(IUserManager::class);
 
 		$this->handler = new UserPlaceholderHandler($this->session, $this->shareManager, $this->request, $this->userManager);
@@ -53,16 +58,17 @@ class UserPlaceholderHandlerTest extends \Test\TestCase {
 		];
 	}
 
-	#[\PHPUnit\Framework\Attributes\DataProvider('optionProvider')]
+	#[DataProvider('optionProvider')]
 	public function testHandle(string|array $option, string|array $expected): void {
 		$this->setUser();
 		$this->assertSame($expected, $this->handler->handle($option));
 	}
 
-	#[\PHPUnit\Framework\Attributes\DataProvider('optionProvider')]
+	#[DataProvider('optionProvider')]
 	public function testHandleNoUser(string|array $option): void {
 		$this->shareManager->expects($this->once())
 			->method('getShareByToken')
+			->with('foo')
 			->willThrowException(new ShareNotFound());
 		$this->assertSame($option, $this->handler->handle($option));
 	}

apps/files_sharing/lib/Command/CleanupRemoteStorages.php

@@ -7,6 +7,7 @@
  */
 namespace OCA\Files_Sharing\Command;
 
+use OCA\Files_Sharing\External\ExternalShareMapper;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Federation\ICloudIdManager;
 use OCP\IDBConnection;
@@ -22,13 +23,13 @@ use Symfony\Component\Console\Output\OutputInterface;
 class CleanupRemoteStorages extends Command {
 
 	public function __construct(
-		protected IDBConnection $connection,
-		private ICloudIdManager $cloudIdManager,
+		protected readonly IDBConnection $connection,
+		private readonly ICloudIdManager $cloudIdManager,
 	) {
 		parent::__construct();
 	}
 
-	protected function configure() {
+	protected function configure(): void {
 		$this
 			->setName('sharing:cleanup-remote-storages')
 			->setDescription('Cleanup shared storage entries that have no matching entry in the shares_external table')
@@ -74,10 +75,11 @@ class CleanupRemoteStorages extends Command {
 				}
 			}
 		}
-		return 0;
+
+		return Command::SUCCESS;
 	}
 
-	public function countFiles($numericId, OutputInterface $output) {
+	public function countFiles($numericId, OutputInterface $output): void {
 		$queryBuilder = $this->connection->getQueryBuilder();
 		$queryBuilder->select($queryBuilder->func()->count('fileid'))
 			->from('filecache')
@@ -92,7 +94,7 @@ class CleanupRemoteStorages extends Command {
 		$output->writeln("$count files can be deleted for storage $numericId");
 	}
 
-	public function deleteStorage($id, $numericId, OutputInterface $output) {
+	public function deleteStorage($id, $numericId, OutputInterface $output): void {
 		$queryBuilder = $this->connection->getQueryBuilder();
 		$queryBuilder->delete('storages')
 			->where($queryBuilder->expr()->eq(
@@ -106,7 +108,7 @@ class CleanupRemoteStorages extends Command {
 		$this->deleteFiles($numericId, $output);
 	}
 
-	public function deleteFiles($numericId, OutputInterface $output) {
+	public function deleteFiles($numericId, OutputInterface $output): void {
 		$queryBuilder = $this->connection->getQueryBuilder();
 		$queryBuilder->delete('filecache')
 			->where($queryBuilder->expr()->eq(
@@ -119,7 +121,11 @@ class CleanupRemoteStorages extends Command {
 		$output->writeln("deleted $count files");
 	}
 
-	public function getRemoteStorages() {
+	/**
+	 * @return array<string, int>
+	 * @throws \OCP\DB\Exception
+	 */
+	private function getRemoteStorages(): array {
 		$queryBuilder = $this->connection->getQueryBuilder();
 		$queryBuilder->select(['id', 'numeric_id'])
 			->from('storages')
@@ -148,7 +154,10 @@ class CleanupRemoteStorages extends Command {
 		return $remoteStorages;
 	}
 
-	public function getRemoteShareIds() {
+	/**
+	 * @return array<string, string>
+	 */
+	private function getRemoteShareIds(): array {
 		$queryBuilder = $this->connection->getQueryBuilder();
 		$queryBuilder->select(['id', 'share_token', 'owner', 'remote'])
 			->from('share_external');
@@ -159,7 +168,6 @@ class CleanupRemoteStorages extends Command {
 		while ($row = $result->fetchAssociative()) {
 			$cloudId = $this->cloudIdManager->getCloudId($row['owner'], $row['remote']);
 			$remote = $cloudId->getRemote();
-
 			$remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $remote);
 		}
 		$result->closeCursor();

apps/files_sharing/lib/Controller/RemoteController.php

@@ -17,8 +17,6 @@ use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
 use OCP\Files\IRootFolder;
-use OCP\Files\NotFoundException;
-use OCP\Files\NotPermittedException;
 use OCP\IRequest;
 use Psr\Log\LoggerInterface;
 
@@ -114,7 +112,7 @@ class RemoteController extends OCSController {
 
 		try {
 			$mountPointNode = $userFolder->get($share->getMountpoint());
-		} catch (NotPermittedException|NotFoundException) {
+		} catch (\Exception) {
 			return $share->jsonSerialize();
 		}
 
@@ -182,7 +180,7 @@ class RemoteController extends OCSController {
 			throw new OCSNotFoundException('Share does not exist');
 		}
 
-		$mountPoint = '/' . $this->userId . '/files' . $shareInfo->getMountPoint();
+		$mountPoint = '/' . $this->userId . '/files' . $shareInfo->getMountpoint();
 
 		if ($this->externalManager->removeShare($mountPoint) === true) {
 			return new DataResponse();

apps/files_sharing/lib/External/ExternalShare.php

@@ -31,8 +31,8 @@ use OCP\Share\IShare;
  * @method void setRemoteId(string $remoteId)
  * @method string getShareToken()
  * @method void setShareToken(string $shareToken)
- * @method string getPassword()
- * @method void setPassword(string $password)
+ * @method string|null getPassword()
+ * @method void setPassword(?string $password)
  * @method string getName()
  * @method string getOwner()
  * @method void setOwner(string $owner)
@@ -97,7 +97,7 @@ class ExternalShare extends Entity implements \JsonSerializable {
 		$parent = $this->getParent();
 		return [
 			'id' => $this->getId(),
-			'parent' => $parent === '-1' ? null : $parent,
+			'parent' => $parent,
 			'share_type' => $this->getShareType() ?? IShare::TYPE_USER, // unfortunately nullable on the DB level, but never null.
 			'remote' => $this->getRemote(),
 			'remote_id' => $this->getRemoteId(),
@@ -106,7 +106,7 @@ class ExternalShare extends Entity implements \JsonSerializable {
 			'owner' => $this->getOwner(),
 			'user' => $this->getUser(),
 			'mountpoint' => $this->getMountpoint(),
-			'accepted' => $this->getAccepted() === IShare::STATUS_ACCEPTED,
+			'accepted' => $this->getAccepted(),
 
 			// Added later on
 			'file_id' => null,

apps/files_sharing/lib/External/ExternalShareMapper.php

@@ -75,10 +75,6 @@ class ExternalShareMapper extends QBMapper {
 		return $this->findEntity($qb);
 	}
 
-	public function get() {
-
-	}
-
 	public function getByMountPointAndUser(string $mountPoint, IUser $user) {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')
@@ -151,6 +147,17 @@ class ExternalShareMapper extends QBMapper {
 		}
 	}
 
+	/**
+	 * @return \Generator<ExternalShare>
+	 */
+	public function getAllShares(): \Generator {
+		$qb = $this->db->getQueryBuilder();
+		$qb->select('*')
+			->from(self::TABLE_NAME);
+
+		return $this->yieldEntities($qb);
+	}
+
 	/**
 	 * Return a list of shares for the user.
 	 *
@@ -201,4 +208,10 @@ class ExternalShareMapper extends QBMapper {
 		}
 		return array_values($shares);
 	}
+
+	public function deleteAll(): void {
+		$qb = $this->db->getQueryBuilder();
+		$qb->delete('share_external')
+			->executeStatement();
+	}
 }

apps/files_sharing/lib/ResponseDefinitions.php

@@ -81,7 +81,7 @@ namespace OCA\Files_Sharing;
  * }
  *
  * @psalm-type Files_SharingRemoteShare = array{
- *     accepted: bool,
+ *     accepted: int,
  *     file_id: int|null,
  *     id: string,
  *     mimetype: string|null,
@@ -89,7 +89,7 @@ namespace OCA\Files_Sharing;
  *     mtime: int|null,
  *     name: string,
  *     owner: string,
- *     parent: string|null,
+ *     parent: string,
  *     permissions: int|null,
  *     remote: string,
  *     remote_id: string,

apps/files_sharing/tests/CapabilitiesTest.php

@@ -19,11 +19,9 @@ use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IDateTimeZone;
 use OCP\IGroupManager;
-use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
-use OCP\Mail\IMailer;
 use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
 use OCP\Share\IProviderFactory;

apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php

@@ -15,21 +15,17 @@ use OCP\Federation\ICloudIdManager;
 use OCP\IDBConnection;
 use OCP\Server;
 use OCP\Snowflake\IGenerator;
+use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Test\TestCase;
 
-/**
- * Class CleanupRemoteStoragesTest
- *
- *
- * @package OCA\Files_Sharing\Tests\Command
- */
-#[\PHPUnit\Framework\Attributes\Group('DB')]
+#[Group('DB')]
 class CleanupRemoteStoragesTest extends TestCase {
 
 	protected IDBConnection $connection;
+	protected ExternalShareMapper $mapper;
 	protected CleanupRemoteStorages $command;
 	private ICloudIdManager&MockObject $cloudIdManager;
 
@@ -47,6 +43,7 @@ class CleanupRemoteStoragesTest extends TestCase {
 		parent::setUp();
 
 		$this->connection = Server::get(IDBConnection::class);
+		$this->mapper = Server::get(ExternalShareMapper::class);
 
 		$storageQuery = Server::get(IDBConnection::class)->getQueryBuilder();
 		$storageQuery->insert('storages')
@@ -74,7 +71,7 @@ class CleanupRemoteStoragesTest extends TestCase {
 				$externalShare->setOwner('irrelevant');
 				$externalShare->setUser($storage['user']);
 				$externalShare->setMountpoint('irrelevant');
-				Server::get(ExternalShareMapper::class)->insert($externalShare);
+				$this->mapper->insert($externalShare);
 			}
 
 			if (isset($storage['files_count'])) {

build/integration/features/bootstrap/AppConfiguration.php

@@ -7,6 +7,7 @@
  */
 use Behat\Behat\Hook\Scope\AfterScenarioScope;
 use Behat\Behat\Hook\Scope\BeforeScenarioScope;
+use Behat\Gherkin\Node\TableNode;
 use PHPUnit\Framework\Assert;
 use Psr\Http\Message\ResponseInterface;
 
@@ -19,8 +20,8 @@ trait AppConfiguration {
 	/** @var ResponseInterface */
 	private $response = null;
 
-	abstract public function sendingTo($verb, $url);
-	abstract public function sendingToWith($verb, $url, $body);
+	abstract public function sendingTo(string $verb, string $url);
+	abstract public function sendingToWith(string $verb, string $url, ?TableNode $body);
 	abstract public function theOCSStatusCodeShouldBe($statusCode);
 	abstract public function theHTTPStatusCodeShouldBe($statusCode);
 

build/integration/features/bootstrap/BasicStructure.php

@@ -74,26 +74,23 @@ trait BasicStructure {
 
 	/**
 	 * @Given /^using api version "(\d+)"$/
-	 * @param string $version
 	 */
-	public function usingApiVersion($version) {
+	public function usingApiVersion(int $version): void {
 		$this->apiVersion = (int)$version;
 	}
 
 	/**
 	 * @Given /^As an "([^"]*)"$/
-	 * @param string $user
 	 */
-	public function asAn($user) {
+	public function asAn(string $user): void {
 		$this->currentUser = $user;
 	}
 
 	/**
 	 * @Given /^Using server "(LOCAL|REMOTE)"$/
-	 * @param string $server
 	 * @return string Previous used server
 	 */
-	public function usingServer($server) {
+	public function usingServer(string $server): string {
 		$previousServer = $this->currentServer;
 		if ($server === 'LOCAL') {
 			$this->baseUrl = $this->localBaseUrl;
@@ -108,21 +105,16 @@ trait BasicStructure {
 
 	/**
 	 * @When /^sending "([^"]*)" to "([^"]*)"$/
-	 * @param string $verb
-	 * @param string $url
 	 */
-	public function sendingTo($verb, $url) {
+	public function sendingTo(string $verb, string $url): void {
 		$this->sendingToWith($verb, $url, null);
 	}
 
 	/**
 	 * Parses the xml or json answer to get ocs response which doesn't match with
 	 * http one in v1 of the api.
-	 *
-	 * @param ResponseInterface $response
-	 * @return string
 	 */
-	public function getOCSResponseCode($response): int {
+	public function getOCSResponseCode(ResponseInterface $response): int {
 		if ($response === null) {
 			throw new \RuntimeException('No response available');
 		}
@@ -141,11 +133,8 @@ trait BasicStructure {
 
 	/**
 	 * This function is needed to use a vertical fashion in the gherkin tables.
-	 *
-	 * @param array $arrayOfArrays
-	 * @return array
 	 */
-	public function simplifyArray($arrayOfArrays) {
+	public function simplifyArray(array $arrayOfArrays): array {
 		$a = array_map(function ($subArray) {
 			return $subArray[0];
 		}, $arrayOfArrays);
@@ -154,11 +143,8 @@ trait BasicStructure {
 
 	/**
 	 * @When /^sending "([^"]*)" to "([^"]*)" with$/
-	 * @param string $verb
-	 * @param string $url
-	 * @param TableNode $body
 	 */
-	public function sendingToWith($verb, $url, $body) {
+	public function sendingToWith(string $verb, string $url, ?TableNode $body): void {
 		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php" . $url;
 		$client = new Client();
 		$options = [];
@@ -191,13 +177,7 @@ trait BasicStructure {
 		}
 	}
 
-	/**
-	 * @param string $verb
-	 * @param string $url
-	 * @param TableNode|array|null $body
-	 * @param array $headers
-	 */
-	protected function sendRequestForJSON(string $verb, string $url, $body = null, array $headers = []): void {
+	protected function sendRequestForJSON(string $verb, string $url, TableNode|array|null $body = null, array $headers = []): void {
 		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php" . $url;
 		$client = new Client();
 		$options = [];

build/integration/features/bootstrap/FederationContext.php

@@ -33,7 +33,7 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	 * The server is started also after the scenarios to ensure that it is
 	 * properly cleaned up if stopped.
 	 */
-	public function startFederatedServer() {
+	public function startFederatedServer(): void {
 		if (self::$phpFederatedServerPid !== '') {
 			return;
 		}
@@ -46,7 +46,7 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @BeforeScenario
 	 */
-	public function cleanupRemoteStorages() {
+	public function cleanupRemoteStorages(): void {
 		// Ensure that dangling remote storages from previous tests will not
 		// interfere with the current scenario.
 		// The storages must be cleaned before each scenario; they can not be
@@ -59,13 +59,10 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @Given /^User "([^"]*)" from server "(LOCAL|REMOTE)" shares "([^"]*)" with user "([^"]*)" from server "(LOCAL|REMOTE)"$/
 	 *
-	 * @param string $sharerUser
-	 * @param string $sharerServer "LOCAL" or "REMOTE"
-	 * @param string $sharerPath
-	 * @param string $shareeUser
-	 * @param string $shareeServer "LOCAL" or "REMOTE"
+	 * @param 'LOCAL'|'REMOTE' $sharerServer "LOCAL" or "REMOTE"
+	 * @param 'LOCAL'|'REMOTE' $shareeServer
 	 */
-	public function federateSharing($sharerUser, $sharerServer, $sharerPath, $shareeUser, $shareeServer) {
+	public function federateSharing(string $sharerUser, string $sharerServer, string $sharerPath, string $shareeUser, string $shareeServer): void {
 		if ($shareeServer == 'REMOTE') {
 			$shareWith = "$shareeUser@" . substr($this->remoteBaseUrl, 0, -4);
 		} else {
@@ -80,13 +77,10 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @Given /^User "([^"]*)" from server "(LOCAL|REMOTE)" shares "([^"]*)" with group "([^"]*)" from server "(LOCAL|REMOTE)"$/
 	 *
-	 * @param string $sharerUser
-	 * @param string $sharerServer "LOCAL" or "REMOTE"
-	 * @param string $sharerPath
-	 * @param string $shareeUser
-	 * @param string $shareeServer "LOCAL" or "REMOTE"
+	 * @param 'LOCAL'|'REMOTE' $sharerServer "LOCAL" or "REMOTE"
+	 * @param 'LOCAL'|'REMOTE' $shareeServer
 	 */
-	public function federateGroupSharing($sharerUser, $sharerServer, $sharerPath, $shareeGroup, $shareeServer) {
+	public function federateGroupSharing(string $sharerUser, string $sharerServer, string $sharerPath, string $shareeGroup, string $shareeServer) {
 		if ($shareeServer == 'REMOTE') {
 			$shareWith = "$shareeGroup@" . substr($this->remoteBaseUrl, 0, -4);
 		} else {
@@ -99,11 +93,8 @@ class FederationContext implements Context, SnippetAcceptingContext {
 
 	/**
 	 * @Then remote share :count is returned with
-	 *
-	 * @param int $number
-	 * @param TableNode $body
 	 */
-	public function remoteShareXIsReturnedWith(int $number, TableNode $body) {
+	public function remoteShareXIsReturnedWith(int $number, TableNode $body): void {
 		$this->theHTTPStatusCodeShouldBe('200');
 		$this->theOCSStatusCodeShouldBe('100');
 
@@ -130,16 +121,15 @@ class FederationContext implements Context, SnippetAcceptingContext {
 
 	/**
 	 * @When /^User "([^"]*)" from server "(LOCAL|REMOTE)" accepts last pending share$/
-	 * @param string $user
-	 * @param string $server
 	 */
-	public function acceptLastPendingShare($user, $server) {
+	public function acceptLastPendingShare(string $user, string $server): void {
 		$previous = $this->usingServer($server);
 		$this->asAn($user);
 		$this->sendingToWith('GET', '/apps/files_sharing/api/v1/remote_shares/pending', null);
 		$this->theHTTPStatusCodeShouldBe('200');
 		$this->theOCSStatusCodeShouldBe('100');
-		$share_id = simplexml_load_string($this->response->getBody())->data[0]->element[0]->id;
+		$shares = simplexml_load_string($this->response->getBody())->data[0]->element;
+		$share_id = $shares[count($shares) - 1]->id;
 		$this->sendingToWith('POST', "/apps/files_sharing/api/v1/remote_shares/pending/{$share_id}", null);
 		$this->theHTTPStatusCodeShouldBe('200');
 		$this->theOCSStatusCodeShouldBe('100');
@@ -150,9 +140,8 @@ class FederationContext implements Context, SnippetAcceptingContext {
 
 	/**
 	 * @When /^user "([^"]*)" deletes last accepted remote share$/
-	 * @param string $user
 	 */
-	public function deleteLastAcceptedRemoteShare($user) {
+	public function deleteLastAcceptedRemoteShare(string $user): void {
 		$this->asAn($user);
 		$this->sendingToWith('DELETE', '/apps/files_sharing/api/v1/remote_shares/' . $this->lastAcceptedRemoteShareId, null);
 	}
@@ -160,7 +149,7 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @When /^remote server is stopped$/
 	 */
-	public function remoteServerIsStopped() {
+	public function remoteServerIsStopped(): void {
 		if (self::$phpFederatedServerPid === '') {
 			return;
 		}
@@ -173,7 +162,7 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @BeforeScenario @TrustedFederation
 	 */
-	public function theServersAreTrustingEachOther() {
+	public function theServersAreTrustingEachOther(): void {
 		$this->asAn('admin');
 		// Trust the remote server on the local server
 		$this->usingServer('LOCAL');
@@ -190,7 +179,7 @@ class FederationContext implements Context, SnippetAcceptingContext {
 	/**
 	 * @AfterScenario @TrustedFederation
 	 */
-	public function theServersAreNoLongerTrustingEachOther() {
+	public function theServersAreNoLongerTrustingEachOther(): void {
 		$this->asAn('admin');
 		// Untrust the remote servers on the local server
 		$this->usingServer('LOCAL');
@@ -213,7 +202,7 @@ class FederationContext implements Context, SnippetAcceptingContext {
 		}
 	}
 
-	protected function resetAppConfigs() {
+	protected function resetAppConfigs(): void {
 		$this->deleteServerConfig('files_sharing', 'incoming_server2server_group_share_enabled');
 		$this->deleteServerConfig('files_sharing', 'outgoing_server2server_group_share_enabled');
 		$this->deleteServerConfig('files_sharing', 'federated_trusted_share_auto_accept');

build/integration/features/bootstrap/Sharing.php

@@ -17,27 +17,19 @@ require __DIR__ . '/autoload.php';
 trait Sharing {
 	use Provisioning;
 
-	/** @var int */
-	private $sharingApiVersion = 1;
-
-	/** @var SimpleXMLElement */
-	private $lastShareData = null;
+	private int $sharingApiVersion = 1;
+	private ?SimpleXMLElement $lastShareData = null;
 
 	/** @var SimpleXMLElement[] */
-	private $storedShareData = [];
-
-	/** @var int */
-	private $savedShareId = null;
-
+	private array $storedShareData = [];
+	private ?string $savedShareId = null;
 	/** @var ResponseInterface */
 	private $response;
 
 	/**
 	 * @Given /^as "([^"]*)" creating a share with$/
-	 * @param string $user
-	 * @param TableNode|null $body
 	 */
-	public function asCreatingAShareWith($user, $body) {
+	public function asCreatingAShareWith(string $user, ?TableNode $body): void {
 		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/files_sharing/api/v{$this->sharingApiVersion}/shares";
 		$client = new Client();
 		$options = [
@@ -78,29 +70,28 @@ trait Sharing {
 	/**
 	 * @When /^save the last share data as "([^"]*)"$/
 	 */
-	public function saveLastShareData($name) {
+	public function saveLastShareData(string $name): void {
 		$this->storedShareData[$name] = $this->lastShareData;
 	}
 
 	/**
 	 * @When /^restore the last share data from "([^"]*)"$/
 	 */
-	public function restoreLastShareData($name) {
+	public function restoreLastShareData(string $name): void {
 		$this->lastShareData = $this->storedShareData[$name];
 	}
 
 	/**
 	 * @When /^creating a share with$/
-	 * @param TableNode|null $body
 	 */
-	public function creatingShare($body) {
+	public function creatingShare(?TableNode $body): void {
 		$this->asCreatingAShareWith($this->currentUser, $body);
 	}
 
 	/**
 	 * @When /^accepting last share$/
 	 */
-	public function acceptingLastShare() {
+	public function acceptingLastShare(): void {
 		$share_id = $this->lastShareData->data[0]->id;
 		$url = "/apps/files_sharing/api/v{$this->sharingApiVersion}/shares/pending/$share_id";
 		$this->sendingToWith('POST', $url, null);
@@ -110,10 +101,8 @@ trait Sharing {
 
 	/**
 	 * @When /^user "([^"]*)" accepts last share$/
-	 *
-	 * @param string $user
 	 */
-	public function userAcceptsLastShare(string $user) {
+	public function userAcceptsLastShare(string $user): void {
 		// "As userXXX" and "user userXXX accepts last share" steps are not
 		// expected to be used in the same scenario, but restore the user just
 		// in case.
@@ -133,7 +122,7 @@ trait Sharing {
 	/**
 	 * @Then /^last link share can be downloaded$/
 	 */
-	public function lastLinkShareCanBeDownloaded() {
+	public function lastLinkShareCanBeDownloaded(): void {
 		if (count($this->lastShareData->data->element) > 0) {
 			$url = $this->lastShareData->data[0]->url;
 		} else {
@@ -146,7 +135,7 @@ trait Sharing {
 	/**
 	 * @Then /^last share can be downloaded$/
 	 */
-	public function lastShareCanBeDownloaded() {
+	public function lastShareCanBeDownloaded(): void {
 		if (count($this->lastShareData->data->element) > 0) {
 			$token = $this->lastShareData->data[0]->token;
 		} else {
@@ -533,7 +522,7 @@ trait Sharing {
 	/**
 	 * @Then the list of returned shares has :count shares
 	 */
-	public function theListOfReturnedSharesHasShares(int $count) {
+	public function theListOfReturnedSharesHasShares(int $count): void {
 		$this->theHTTPStatusCodeShouldBe('200');
 		$this->theOCSStatusCodeShouldBe('100');
 

build/integration/features/bootstrap/WebDav.php

@@ -33,14 +33,14 @@ trait WebDav {
 	/**
 	 * @Given /^using dav path "([^"]*)"$/
 	 */
-	public function usingDavPath($davPath) {
+	public function usingDavPath(string $davPath): void {
 		$this->davPath = $davPath;
 	}
 
 	/**
 	 * @Given /^using old dav path$/
 	 */
-	public function usingOldDavPath() {
+	public function usingOldDavPath(): void {
 		$this->davPath = 'remote.php/webdav';
 		$this->usingOldDavPath = true;
 	}
@@ -48,7 +48,7 @@ trait WebDav {
 	/**
 	 * @Given /^using new dav path$/
 	 */
-	public function usingNewDavPath() {
+	public function usingNewDavPath(): void {
 		$this->davPath = 'remote.php/dav';
 		$this->usingOldDavPath = false;
 	}
@@ -56,7 +56,7 @@ trait WebDav {
 	/**
 	 * @Given /^using new public dav path$/
 	 */
-	public function usingNewPublicDavPath() {
+	public function usingNewPublicDavPath(): void {
 		$this->davPath = 'public.php/dav';
 		$this->usingOldDavPath = false;
 	}