Apply review suggestions

Signed-off-by: Louis Chemineau <louis@chmn.me>

9 files changed, 48 insertions, 50 deletions

diff --git a/lib/Album/AlbumMapper.php b/lib/Album/AlbumMapper.php
index 2b92c3b8..c788df8f 100644
--- a/lib/Album/AlbumMapper.php
+++ b/lib/Album/AlbumMapper.php
@@ -151,13 +151,12 @@ class AlbumMapper {
 		$query->executeStatement();
 	}
 
-	public function setLocation(int $id, string $newLocation): string {
+	public function setLocation(int $id, string $newLocation): void {
 		$query = $this->connection->getQueryBuilder();
 		$query->update("photos_albums")
 			->set("location", $query->createNamedParameter($newLocation))
 			->where($query->expr()->eq('album_id', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT)));
 		$query->executeStatement();
-		return $newLocation;
 	}
 
 	public function delete(int $id): void {
@@ -331,7 +330,7 @@ class AlbumMapper {
 
 	/**
 	 * @param int $albumId
-	 * @param array{'id': string, 'label': string, 'type': int} $collaborators
+	 * @param array{'id': string, 'type': int} $collaborators
 	 */
 	public function setCollaborators(int $albumId, array $collaborators): void {
 		$existingCollaborators = $this->getCollaborators($albumId);
diff --git a/lib/Controller/PublicAlbumController.php b/lib/Controller/PublicAlbumController.php
index c7143363..8130506d 100644
--- a/lib/Controller/PublicAlbumController.php
+++ b/lib/Controller/PublicAlbumController.php
@@ -35,7 +35,6 @@ use OCP\AppFramework\Http\TemplateResponse;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\IRequest;
-use OCP\ISession;
 use OCP\Util;
 
 class PublicAlbumController extends Controller {
@@ -46,13 +45,12 @@ class PublicAlbumController extends Controller {
 
 	public function __construct(
 		IRequest $request,
-		ISession $session,
 		IAppManager $appManager,
 		IEventDispatcher $eventDispatcher,
 		UserConfigService $userConfig,
 		IInitialState $initialState,
 	) {
-		parent::__construct(Application::APP_ID, $request, $session);
+		parent::__construct(Application::APP_ID, $request);
 
 		$this->appManager = $appManager;
 		$this->eventDispatcher = $eventDispatcher;
@@ -61,14 +59,8 @@ class PublicAlbumController extends Controller {
 	}
 
 	/**
-	 * Your normal controller function. The following annotation will allow guests
-	 * to open the page as well
-	 *
 	 * @PublicPage
-	 * @NoAdminRequired
 	 * @NoCSRFRequired
-	 *
-	 * @return TemplateResponse
 	 */
 	public function get(): TemplateResponse {
 		$this->eventDispatcher->dispatch(LoadSidebar::class, new LoadSidebar());
diff --git a/lib/Sabre/Album/AlbumRoot.php b/lib/Sabre/Album/AlbumRoot.php
index e2fe6c9f..f0445d85 100644
--- a/lib/Sabre/Album/AlbumRoot.php
+++ b/lib/Sabre/Album/AlbumRoot.php
@@ -84,8 +84,8 @@ class AlbumRoot implements ICollection, ICopyTarget {
 	 * We cannot create files in an Album
 	 * We add the file to the default Photos folder and then link it there.
 	 *
-	 * @param [type] $name
-	 * @param [type] $data
+	 * @param string $name
+	 * @param null|resource|string $data
 	 * @return void
 	 */
 	public function createFile($name, $data = null) {
@@ -218,9 +218,9 @@ class AlbumRoot implements ICollection, ICopyTarget {
 	}
 
 	/**
-	 * @return array{'id': string, 'label': string, 'type': int}
+	 * @return array{array{'nc:collaborator': array{'id': string, 'label': string, 'type': int}}}
 	 */
-	public function getCollaborators() {
+	public function getCollaborators(): array {
 		return array_map(
 			fn (array $collaborator) => [ 'nc:collaborator' => $collaborator ],
 			$this->albumMapper->getCollaborators($this->album->getAlbum()->getId()),
@@ -229,9 +229,9 @@ class AlbumRoot implements ICollection, ICopyTarget {
 
 	/**
 	 * @param array{'id': string, 'type': int} $collaborators
-	 * @return array{'id': string, 'label': string, 'type': int}
+	 * @return array{array{'nc:collaborator': array{'id': string, 'label': string, 'type': int}}}
 	 */
-	public function setCollaborators($collaborators) {
+	public function setCollaborators($collaborators): array {
 		$this->albumMapper->setCollaborators($this->getAlbum()->getAlbum()->getId(), $collaborators);
 		return $this->getCollaborators();
 	}
diff --git a/lib/Sabre/Album/PublicAlbumRoot.php b/lib/Sabre/Album/PublicAlbumRoot.php
index 8ca2e1e6..0a56381d 100644
--- a/lib/Sabre/Album/PublicAlbumRoot.php
+++ b/lib/Sabre/Album/PublicAlbumRoot.php
@@ -26,6 +26,7 @@ namespace OCA\Photos\Sabre\Album;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\Conflict;
 use OCP\Files\Folder;
+use Sabre\DAV\INode;
 
 class PublicAlbumRoot extends AlbumRoot {
 	/**
@@ -42,20 +43,22 @@ class PublicAlbumRoot extends AlbumRoot {
 		throw new Forbidden('Not allowed to rename a public album');
 	}
 
-	// TODO: uncomment else it is a security hole.
-	// public function copyInto($targetName, $sourcePath, INode $sourceNode): bool {
-	//  throw new Forbidden('Not allowed to copy into a public album');
-	// }
+	public function copyInto($targetName, $sourcePath, INode $sourceNode): bool {
+	 throw new Forbidden('Not allowed to copy into a public album');
+	}
 
 	/**
 	 * We cannot create files in an Album
 	 * We add the file to the default Photos folder and then link it there.
 	 *
-	 * @param [type] $name
-	 * @param [type] $data
-	 * @return void
+	 * @param string $name
+	 * @param null|resource|string $data
+	 * @return null
 	 */
 	public function createFile($name, $data = null) {
+		// TODO: implement public album upload
+		throw new Forbidden('Not allowed to create a file in a public album');
+
 		try {
 			$albumOwner = $this->album->getAlbum()->getUserId();
 			$photosLocation = $this->userConfigService->getConfigForUser($albumOwner, 'photosLocation');
@@ -78,8 +81,10 @@ class PublicAlbumRoot extends AlbumRoot {
 		}
 	}
 
-
 	protected function addFile(int $sourceId, string $ownerUID): bool {
+		// TODO: implement public album upload
+		throw new Forbidden('Not allowed to create a file in a public album');
+
 		if (in_array($sourceId, $this->album->getFileIds())) {
 			throw new Conflict("File $sourceId is already in the folder");
 		}
@@ -89,7 +94,8 @@ class PublicAlbumRoot extends AlbumRoot {
 	}
 
 	// Do not reveal collaborators for public albums.
-	public function getCollaborators() {
+	public function getCollaborators(): array {
+		/** @var array{array{'nc:collaborator': array{'id': string, 'label': string, 'type': int}}} */
 		return [];
 	}
 }
diff --git a/lib/Sabre/Album/PublicAlbumsHome.php b/lib/Sabre/Album/PublicAlbumsHome.php
index f7d7e033..5b03e33c 100644
--- a/lib/Sabre/Album/PublicAlbumsHome.php
+++ b/lib/Sabre/Album/PublicAlbumsHome.php
@@ -65,12 +65,12 @@ class PublicAlbumsHome extends AlbumsHome {
 	}
 
 	public function getChild($name) {
-		$basicAuth = $this->server->httpRequest->getHeader('Authorization');
+		$basicAuth = $this->server->httpRequest->getHeader('Authorization') ?? 'Basic ';
 		[, $base64Token] = explode('Basic ', $basicAuth);
 		$token = \base64_decode($base64Token);
 		$albums = $this->albumMapper->getSharedAlbumsForCollaboratorWithFiles($token, AlbumMapper::TYPE_LINK);
 
-		array_filter($albums, fn ($album) => $album->getAlbum()->getUserId() === $this->user->getUid());
+		$albums = array_filter($albums, fn ($album) => $album->getAlbum()->getUserId() === $this->user->getUid());
 
 		if (count($albums) !== 1) {
 			throw new NotFound();
diff --git a/lib/Sabre/Album/SharedAlbumRoot.php b/lib/Sabre/Album/SharedAlbumRoot.php
index 607a009f..dedf569b 100644
--- a/lib/Sabre/Album/SharedAlbumRoot.php
+++ b/lib/Sabre/Album/SharedAlbumRoot.php
@@ -60,8 +60,11 @@ class SharedAlbumRoot extends AlbumRoot {
 		return true;
 	}
 
-	// Do not reveal collaborators for shared albums.
-	public function getCollaborators() {
+	/**
+	 * Do not reveal collaborators for shared albums.
+	 */
+	public function getCollaborators(): array {
+		/** @var array{array{'nc:collaborator': array{'id': string, 'label': string, 'type': int}}} */
 		return [];
 	}
 }
diff --git a/lib/Service/UserConfigService.php b/lib/Service/UserConfigService.php
index cf717d83..0a3ced64 100644
--- a/lib/Service/UserConfigService.php
+++ b/lib/Service/UserConfigService.php
@@ -47,12 +47,12 @@ class UserConfigService {
 		$this->userSession = $userSession;
 	}
 
-	public function getUserConfig(string $key) {
+	public function getUserConfig(string $key): string {
 		$user = $this->userSession->getUser();
 		return $this->getConfigForUser($user->getUid(), $key);
 	}
 
-	public function getConfigForUser(string $userId, string $key) {
+	public function getConfigForUser(string $userId, string $key): string {
 		if (!in_array($key, array_keys(self::DEFAULT_CONFIGS))) {
 			throw new Exception('Unknown user config key');
 		}
diff --git a/src/Photos.vue b/src/Photos.vue
index 909b02ae..23b8af48 100644
--- a/src/Photos.vue
+++ b/src/Photos.vue
@@ -178,14 +178,14 @@ export default {
 				}), {
 					scope: generateUrl('/apps/photos'),
 				}).then(registration => {
-					console.debug('SW registered: ', registration)
+					logger.debug('SW registered: ', registration)
 				}).catch(registrationError => {
-					console.error('SW registration failed: ', registrationError)
+					logger.error('SW registration failed: ', registrationError)
 				})
 
 			})
 		} else {
-			console.debug('Service Worker is not enabled on this browser.')
+			logger.debug('Service Worker is not enabled on this browser.')
 		}
 	},
 
diff --git a/src/PhotosPublic.vue b/src/PhotosPublic.vue
index 18d008ef..493ddaa0 100644
--- a/src/PhotosPublic.vue
+++ b/src/PhotosPublic.vue
@@ -1,7 +1,7 @@
 <!--
- - @copyright Copyright (c) 2019 John Molakvoæ <skjnldsv@protonmail.com>
+ - @copyright Copyright (c) 2022 Louis Chmn <louis@chmn.me>
  -
- - @author John Molakvoæ <skjnldsv@protonmail.com>
+ - @author Louis Chmn <louis@chmn.me>
  -
  - @license AGPL-3.0-or-later
  -
@@ -41,6 +41,7 @@ import { generateUrl } from '@nextcloud/router'
 
 import { NcContent, NcAppContent } from '@nextcloud/vue'
 
+import logger from './services/logger.js'
 import svgplaceholder from './assets/file-placeholder.svg'
 import imgplaceholder from './assets/image.svg'
 import videoplaceholder from './assets/video.svg'
@@ -62,20 +63,17 @@ export default {
 	async beforeMount() {
 		if ('serviceWorker' in navigator) {
 			// Use the window load event to keep the page load performant
-			window.addEventListener('load', () => {
-				navigator.serviceWorker.register(generateUrl('/apps/photos/service-worker.js', {}, {
-					noRewrite: true,
-				}), {
-					scope: '/',
-				}).then(registration => {
-					console.debug('SW registered: ', registration)
-				}).catch(registrationError => {
-					console.error('SW registration failed: ', registrationError)
-				})
-
+			window.addEventListener('load', async () => {
+				try {
+					const url = generateUrl('/apps/photos/service-worker.js', {}, { noRewrite: true })
+					const registration = await navigator.serviceWorker.register(url, { scope: '/' })
+					logger.debug('SW registered: ', registration)
+				} catch (error) {
+					logger.error('SW registration failed: ', error)
+				}
 			})
 		} else {
-			console.debug('Service Worker is not enabled on this browser.')
+			logger.debug('Service Worker is not enabled on this browser.')
 		}
 	},