feat: Add log when file could not be created in an album

Author: artonge

lib/Sabre/Album/AlbumRoot.php

@@ -32,25 +32,29 @@
 use OCP\Files\InvalidDirectoryException;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Exception\Conflict;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\ICollection;
 use Sabre\DAV\ICopyTarget;
 use Sabre\DAV\INode;
 
-class AlbumRoot implements ICollection, ICopyTarget {
+class AlbumRoot implements ICollection, ICopyTarget
+{
 	protected AlbumMapper $albumMapper;
 	protected AlbumWithFiles $album;
 	protected IRootFolder $rootFolder;
 	protected string $userId;
+	private UserConfigService $userConfigService;
 
 	public function __construct(
 		AlbumMapper $albumMapper,
 		AlbumWithFiles $album,
 		IRootFolder $rootFolder,
 		string $userId,
-		UserConfigService $userConfigService
+		UserConfigService $userConfigService,
+		protected LoggerInterface $logger,
 	) {
 		$this->albumMapper = $albumMapper;
 		$this->album = $album;
@@ -62,22 +66,26 @@ public function __construct(
 	/**
 	 * @return void
 	 */
-	public function delete() {
+	public function delete()
+	{
 		$this->albumMapper->delete($this->album->getAlbum()->getId());
 	}
 
-	public function getName(): string {
+	public function getName(): string
+	{
 		return basename($this->album->getAlbum()->getTitle());
 	}
 
 	/**
 	 * @return void
 	 */
-	public function setName($name) {
+	public function setName($name)
+	{
 		$this->albumMapper->rename($this->album->getAlbum()->getId(), $name);
 	}
 
-	protected function getPhotosLocationInfo() {
+	protected function getPhotosLocationInfo()
+	{
 		$photosLocation = $this->userConfigService->getUserConfig('photosLocation');
 		$userFolder = $this->rootFolder->getUserFolder($this->userId);
 		return [$photosLocation, $userFolder];
@@ -91,7 +99,8 @@ protected function getPhotosLocationInfo() {
 	 * @param null|resource|string $data
 	 * @return void
 	 */
-	public function createFile($name, $data = null) {
+	public function createFile($name, $data = null)
+	{
 		try {
 			[$photosLocation, $userFolder] = $this->getPhotosLocationInfo();
 
@@ -121,24 +130,28 @@ public function createFile($name, $data = null) {
 			\header('OC-FileId: ' . $node->getId());
 			return '"' . $node->getEtag() . '"';
 		} catch (\Exception $e) {
+			$this->logger->error('Could not create file', ['exception' => $e]);
 			throw new Forbidden('Could not create file');
 		}
 	}
 
 	/**
 	 * @return never
 	 */
-	public function createDirectory($name) {
+	public function createDirectory($name)
+	{
 		throw new Forbidden('Not allowed to create directories in this folder');
 	}
 
-	public function getChildren(): array {
+	public function getChildren(): array
+	{
 		return array_map(function (AlbumFile $file) {
 			return new AlbumPhoto($this->albumMapper, $this->album->getAlbum(), $file, $this->rootFolder, $this->rootFolder->getUserFolder($this->userId));
 		}, $this->album->getFiles());
 	}
 
-	public function getChild($name): AlbumPhoto {
+	public function getChild($name): AlbumPhoto
+	{
 		foreach ($this->album->getFiles() as $file) {
 			if ($file->getFileId() . "-" . $file->getName() === $name) {
 				return new AlbumPhoto($this->albumMapper, $this->album->getAlbum(), $file, $this->rootFolder, $this->rootFolder->getUserFolder($this->userId));
@@ -147,7 +160,8 @@ public function getChild($name): AlbumPhoto {
 		throw new NotFound("$name not found");
 	}
 
-	public function childExists($name): bool {
+	public function childExists($name): bool
+	{
 		try {
 			$this->getChild($name);
 			return true;
@@ -156,11 +170,13 @@ public function childExists($name): bool {
 		}
 	}
 
-	public function getLastModified(): int {
+	public function getLastModified(): int
+	{
 		return 0;
 	}
 
-	public function copyInto($targetName, $sourcePath, INode $sourceNode): bool {
+	public function copyInto($targetName, $sourcePath, INode $sourceNode): bool
+	{
 		if (!$sourceNode instanceof File) {
 			throw new Forbidden("The source is not a file");
 		}
@@ -175,7 +191,8 @@ public function copyInto($targetName, $sourcePath, INode $sourceNode): bool {
 		return $this->addFile($sourceId, $ownerUID);
 	}
 
-	protected function addFile(int $sourceId, string $ownerUID): bool {
+	protected function addFile(int $sourceId, string $ownerUID): bool
+	{
 		if (in_array($sourceId, $this->album->getFileIds())) {
 			throw new Conflict("File $sourceId is already in the folder");
 		}
@@ -188,11 +205,13 @@ protected function addFile(int $sourceId, string $ownerUID): bool {
 		return false;
 	}
 
-	public function getAlbum(): AlbumWithFiles {
+	public function getAlbum(): AlbumWithFiles
+	{
 		return $this->album;
 	}
 
-	public function getDateRange(): array {
+	public function getDateRange(): array
+	{
 		$earliestDate = null;
 		$latestDate = null;
 
@@ -218,7 +237,8 @@ public function getDateRange(): array {
 	/**
 	 * @return int|null
 	 */
-	public function getCover() {
+	public function getCover()
+	{
 		$children = $this->getChildren();
 
 		if (count($children) > 0) {
@@ -231,7 +251,8 @@ public function getCover() {
 	/**
 	 * @return array{array{'nc:collaborator': array{'id': string, 'label': string, 'type': int}}}
 	 */
-	public function getCollaborators(): array {
+	public function getCollaborators(): array
+	{
 		return array_map(
 			fn (array $collaborator) => [ 'nc:collaborator' => $collaborator ],
 			$this->albumMapper->getCollaborators($this->album->getAlbum()->getId()),
@@ -242,7 +263,8 @@ public function getCollaborators(): array {
 	 * @param array{'id': string, 'type': int} $collaborators
 	 * @return array{array{'nc:collaborator': array{'id': string, 'label': string, 'type': int}}}
 	 */
-	public function setCollaborators($collaborators): array {
+	public function setCollaborators($collaborators): array
+	{
 		$this->albumMapper->setCollaborators($this->getAlbum()->getAlbum()->getId(), $collaborators);
 		return $this->getCollaborators();
 	}

lib/Sabre/Album/AlbumsHome.php

@@ -28,17 +28,12 @@
 use OCA\Photos\Album\AlbumWithFiles;
 use OCA\Photos\Service\UserConfigService;
 use OCP\Files\IRootFolder;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\ICollection;
 
 class AlbumsHome implements ICollection {
-	protected AlbumMapper $albumMapper;
-	protected array $principalInfo;
-	protected string $userId;
-	protected IRootFolder $rootFolder;
-	protected UserConfigService $userConfigService;
-
 	public const NAME = 'albums';
 
 	/**
@@ -47,17 +42,13 @@ class AlbumsHome implements ICollection {
 	protected ?array $children = null;
 
 	public function __construct(
-		array $principalInfo,
-		AlbumMapper $albumMapper,
-		string $userId,
-		IRootFolder $rootFolder,
-		UserConfigService $userConfigService
+		protected array $principalInfo,
+		protected AlbumMapper $albumMapper,
+		protected string $userId,
+		protected IRootFolder $rootFolder,
+		protected UserConfigService $userConfigService,
+		protected LoggerInterface $logger,
 	) {
-		$this->principalInfo = $principalInfo;
-		$this->albumMapper = $albumMapper;
-		$this->userId = $userId;
-		$this->rootFolder = $rootFolder;
-		$this->userConfigService = $userConfigService;
 	}
 
 	/**
@@ -106,7 +97,14 @@ public function getChildren(): array {
 		if ($this->children === null) {
 			$albumInfos = $this->albumMapper->getForUser($this->userId);
 			$this->children = array_map(function (AlbumInfo $albumInfo) {
-				return new AlbumRoot($this->albumMapper, new AlbumWithFiles($albumInfo, $this->albumMapper), $this->rootFolder, $this->userId, $this->userConfigService);
+				return new AlbumRoot(
+					$this->albumMapper,
+					new AlbumWithFiles($albumInfo, $this->albumMapper),
+					$this->rootFolder,
+					$this->userId,
+					$this->userConfigService,
+					$this->logger,
+				);
 			}, $albumInfos);
 		}
 

lib/Sabre/Album/SharedAlbumRoot.php

@@ -27,48 +27,48 @@
 use OCA\Photos\Album\AlbumWithFiles;
 use OCA\Photos\Service\UserConfigService;
 use OCP\Files\IRootFolder;
-use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Exception\Conflict;
 use Sabre\DAV\Exception\Forbidden;
 
-class SharedAlbumRoot extends AlbumRoot {
-	private IUserManager $userManager;
-
+class SharedAlbumRoot extends AlbumRoot
+{
 	public function __construct(
 		AlbumMapper $albumMapper,
 		AlbumWithFiles $album,
 		IRootFolder $rootFolder,
 		string $userId,
 		UserConfigService $userConfigService,
-		IUserManager $userManager
+		LoggerInterface $logger,
 	) {
 		parent::__construct(
 			$albumMapper,
 			$album,
 			$rootFolder,
 			$userId,
 			$userConfigService,
-			$userManager
+			$logger
 		);
-
-		$this->userManager = $userManager;
 	}
 
 	/**
 	 * @return void
 	 */
-	public function delete() {
+	public function delete()
+	{
 		$this->albumMapper->deleteUserFromAlbumCollaboratorsList($this->userId, $this->album->getAlbum()->getId());
 	}
 
 	/**
 	 * @return void
 	 */
-	public function setName($name) {
+	public function setName($name)
+	{
 		throw new Forbidden('Not allowed to rename a shared album');
 	}
 
-	protected function addFile(int $sourceId, string $ownerUID): bool {
+	protected function addFile(int $sourceId, string $ownerUID): bool
+	{
 		if (in_array($sourceId, $this->album->getFileIds())) {
 			throw new Conflict("File $sourceId is already in the folder");
 		}
@@ -84,7 +84,8 @@ protected function addFile(int $sourceId, string $ownerUID): bool {
 	/**
 	 * Return only the owner, and do not reveal other collaborators.
 	 */
-	public function getCollaborators(): array {
+	public function getCollaborators(): array
+	{
 		return [[
 			'nc:collaborator' => [
 				'id' => $this->album->getAlbum()->getUserId(),
@@ -94,7 +95,8 @@ public function getCollaborators(): array {
 		]];
 	}
 
-	public function setCollaborators($collaborators): array {
+	public function setCollaborators($collaborators): array
+	{
 		throw new Forbidden('Not allowed to collaborators to a public album');
 	}
 }

lib/Sabre/Album/SharedAlbumsHome.php

@@ -29,33 +29,31 @@
 use OCP\Files\IRootFolder;
 use OCP\IGroupManager;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Exception\Forbidden;
 
 class SharedAlbumsHome extends AlbumsHome {
-	private IUserManager $userManager;
-	private IGroupManager $groupManager;
-
 	public const NAME = 'sharedalbums';
 
 	public function __construct(
 		array $principalInfo,
 		AlbumMapper $albumMapper,
 		string $userId,
 		IRootFolder $rootFolder,
-		IUserManager $userManager,
-		IGroupManager $groupManager,
-		UserConfigService $userConfigService
+		private IUserManager $userManager,
+		private IGroupManager $groupManager,
+		UserConfigService $userConfigService,
+		LoggerInterface $logger,
+
 	) {
 		parent::__construct(
 			$principalInfo,
 			$albumMapper,
 			$userId,
 			$rootFolder,
-			$userConfigService
+			$userConfigService,
+			$logger,
 		);
-
-		$this->userManager = $userManager;
-		$this->groupManager = $groupManager;
 	}
 
 	/**
@@ -81,7 +79,15 @@ public function getChildren(): array {
 			}
 
 			$this->children = array_map(function (AlbumWithFiles $album) {
-				return new SharedAlbumRoot($this->albumMapper, $album, $this->rootFolder, $this->userId, $this->userConfigService, $this->userManager);
+				return new SharedAlbumRoot(
+					$this->albumMapper,
+					$album,
+					$this->rootFolder,
+					$this->userId,
+					$this->userConfigService,
+					$this->logger,
+					$this->userManager,
+				);
 			}, $albums);
 		}