refactor(navigation-manager): Cleanup implementation and add type hinting

Signed-off-by: Carl Schwan <[email protected]>

Author: CarlSchwan

build/psalm-baseline.xml

@@ -663,7 +663,6 @@
       <code><![CDATA[resolvePath]]></code>
       <code><![CDATA[resolvePath]]></code>
       <code><![CDATA[resolvePath]]></code>
-      <code><![CDATA[resolvePath]]></code>
       <code><![CDATA[touch]]></code>
       <code><![CDATA[unlink]]></code>
     </InternalMethod>

lib/private/NavigationManager.php

@@ -19,16 +19,20 @@
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
 use OCP\Navigation\Events\LoadAdditionalEntriesEvent;
+use Override;
 use Psr\Log\LoggerInterface;
 
 /**
  * Manages the Nextcloud navigation
+ * @psalm-import-type NavigationEntry from INavigationManager
+ * @psalm-import-type NavigationEntryOutput from INavigationManager
  */
 class NavigationManager implements INavigationManager {
+	/** @var array<string, NavigationEntryOutput> */
 	protected array $entries = [];
+	/** @var list<callable(): NavigationEntry> */
 	protected array $closureEntries = [];
-	/** @var string $activeEntry */
-	protected $activeEntry;
+	protected ?string $activeEntry = null;
 	protected array $unreadCounters = [];
 	protected bool $init = false;
 	/** User defined app order (cached for the `add` function) */
@@ -46,10 +50,8 @@ public function __construct(
 	) {
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	public function add($entry) {
+	#[Override]
+	public function add(array|callable $entry): void {
 		if ($entry instanceof \Closure) {
 			$this->closureEntries[] = $entry;
 			return;
@@ -86,7 +88,7 @@ public function add($entry) {
 		$this->updateDefaultEntries();
 	}
 
-	private function updateDefaultEntries() {
+	private function updateDefaultEntries(): void {
 		$defaultEntryId = $this->getDefaultEntryIdForUser($this->userSession->getUser(), false);
 		foreach ($this->entries as $id => $entry) {
 			if ($entry['type'] === 'link') {
@@ -95,9 +97,7 @@ private function updateDefaultEntries() {
 		}
 	}
 
-	/**
-	 * @inheritDoc
-	 */
+	#[Override]
 	public function getAll(string $type = 'link'): array {
 		$this->init();
 
@@ -114,8 +114,8 @@ public function getAll(string $type = 'link'): array {
 	/**
 	 * Sort navigation entries default app is always sorted first, then by order, name and set active flag
 	 *
-	 * @param array $list
-	 * @return array
+	 * @param array<string, NavigationEntryOutput> $list
+	 * @return array<string, NavigationEntryOutput>
 	 */
 	private function proceedNavigation(array $list, string $type): array {
 		uasort($list, function ($a, $b) {
@@ -136,7 +136,7 @@ private function proceedNavigation(array $list, string $type): array {
 
 		if ($type === 'all' || $type === 'link') {
 			// There might be the case that no default app was set, in this case the first app is the default app.
-			// Otherwise the default app is already the ordered first, so setting the default prop will make no difference.
+			// Otherwise, the default app is already the ordered first, so setting the default prop will make no difference.
 			foreach ($list as $index => &$navEntry) {
 				if ($navEntry['type'] === 'link') {
 					$navEntry['default'] = true;
@@ -165,23 +165,19 @@ private function proceedNavigation(array $list, string $type): array {
 	/**
 	 * removes all the entries
 	 */
-	public function clear($loadDefaultLinks = true) {
+	public function clear(bool $loadDefaultLinks = true): void {
 		$this->entries = [];
 		$this->closureEntries = [];
 		$this->init = !$loadDefaultLinks;
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	public function setActiveEntry($appId) {
+	#[Override]
+	public function setActiveEntry(string $appId): void {
 		$this->activeEntry = $appId;
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	public function getActiveEntry() {
+	#[Override]
+	public function getActiveEntry(): ?string {
 		return $this->activeEntry;
 	}
 
@@ -377,31 +373,34 @@ private function init(bool $resolveClosures = true): void {
 		}
 	}
 
-	private function isAdmin() {
+	private function isAdmin(): bool {
 		$user = $this->userSession->getUser();
 		if ($user !== null) {
 			return $this->groupManager->isAdmin($user->getUID());
 		}
 		return false;
 	}
 
-	private function isSubadmin() {
+	private function isSubadmin(): bool {
 		$user = $this->userSession->getUser();
 		if ($user !== null && $this->groupManager instanceof Manager) {
 			return $this->groupManager->getSubAdmin()->isSubAdmin($user);
 		}
 		return false;
 	}
 
+	#[Override]
 	public function setUnreadCounter(string $id, int $unreadCounter): void {
 		$this->unreadCounters[$id] = $unreadCounter;
 	}
 
+	#[Override]
 	public function get(string $id): ?array {
 		$this->init();
 		return $this->entries[$id];
 	}
 
+	#[Override]
 	public function getDefaultEntryIdForUser(?IUser $user = null, bool $withFallbacks = true): string {
 		$this->init();
 		// Disable fallbacks here, as we need to override them with the user defaults if none are configured.
@@ -443,6 +442,7 @@ public function getDefaultEntryIdForUser(?IUser $user = null, bool $withFallback
 		return $withFallbacks ? 'files' : '';
 	}
 
+	#[Override]
 	public function getDefaultEntryIds(bool $withFallbacks = true): array {
 		$this->init();
 		$storedIds = explode(',', $this->config->getSystemValueString('defaultapp', $withFallbacks ? 'dashboard,files' : ''));
@@ -457,6 +457,7 @@ public function getDefaultEntryIds(bool $withFallbacks = true): array {
 		return array_filter($ids);
 	}
 
+	#[Override]
 	public function setDefaultEntryIds(array $ids): void {
 		$this->init();
 		$entryIds = array_keys($this->entries);

lib/public/INavigationManager.php

@@ -10,12 +10,31 @@
 
 namespace OCP;
 
+use OCP\AppFramework\Attribute\Consumable;
+use OCP\AppFramework\Attribute\ExceptionalImplementable;
+
 /**
- * Manages the ownCloud navigation
+ * Manages the Nextcloud navigation
+ *
  * @since 6.0.0
  *
  * @psalm-type NavigationEntry = array{id: string, order: int, href: string, name: string, app?: string, icon?: string, classes?: string, type?: string}
+ * @psalm-type NavigationEntryOutput = array{
+ *     id: string,
+ *     order?: int,
+ *     href: string,
+ *     icon: string,
+ *     type: string,
+ *     name: string,
+ *     app?: string,
+ *     default?: bool,
+ *     active: bool,
+ *     classes: string,
+ *     unread: int,
+ * }
  */
+#[Consumable(since: '6.0.0')]
+#[ExceptionalImplementable(app: 'guest')]
 interface INavigationManager {
 	/**
 	 * Navigation entries of the app navigation
@@ -35,39 +54,43 @@ interface INavigationManager {
 	 */
 	public const TYPE_GUEST = 'guest';
 
+	/**
+	 * All navigation entries
+	 * @since 33.0.0
+	 */
+	public const TYPE_ALL = 'all';
+
 	/**
 	 * Creates a new navigation entry
 	 *
-	 * @param array array|\Closure $entry Array containing: id, name, order, icon and href key
-	 * 					If a menu entry (type = 'link') is added, you shall also set app to the app that added the entry.
-	 *					The use of a closure is preferred, because it will avoid
-	 * 					loading the routing of your app, unless required.
-	 * @psalm-param NavigationEntry|callable():NavigationEntry $entry
+	 * @param NavigationEntry|callable():NavigationEntry $entry If a menu entry (type = 'link') is added, you shall also set app to the app that
+	 *                                                          added the entry. The use of a closure is preferred, because it will avoid loading
+	 *                                                          the routing of your app, unless required.
 	 * @return void
 	 * @since 6.0.0
 	 */
-	public function add($entry);
+	public function add(array|callable $entry): void;
 
 	/**
 	 * Sets the current navigation entry of the currently running app
 	 * @param string $appId id of the app entry to activate (from added $entry)
 	 * @return void
 	 * @since 6.0.0
 	 */
-	public function setActiveEntry($appId);
+	public function setActiveEntry(string $appId): void;
 
 	/**
 	 * Get the current navigation entry of the currently running app
-	 * @return string
+	 * @return ?string
 	 * @since 20.0.0
 	 */
-	public function getActiveEntry();
+	public function getActiveEntry(): ?string;
 
 	/**
 	 * Get a list of navigation entries
 	 *
-	 * @param string $type type of the navigation entries
-	 * @return array
+	 * @param self::TYPE_APPS|self::TYPE_SETTINGS|self::TYPE_GUEST|self::TYPE_ALL $type type of the navigation entries
+	 * @return array<string, NavigationEntryOutput>
 	 * @since 14.0.0
 	 */
 	public function getAll(string $type = self::TYPE_APPS): array;
@@ -92,7 +115,7 @@ public function get(string $id): ?array;
 	/**
 	 * Returns the id of the user's default entry
 	 *
-	 * If `user` is not passed, the currently logged in user will be used
+	 * If `user` is not passed, the currently logged-in user will be used
 	 *
 	 * @param ?IUser $user User to query default entry for
 	 * @param bool $withFallbacks Include fallback values if no default entry was configured manually