Reduce complexity of 'books' route in OpdsController.

oldnomad · Oct 17, 2023 · 5c8ac98 · 5c8ac98
1 parent 7e27a46
commit 5c8ac98
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 36 deletions.
diff --git a/lib/Calibre/Types/CalibreBookCriteria.php b/lib/Calibre/Types/CalibreBookCriteria.php
@@ -6,6 +6,8 @@
 
 namespace OCA\Calibre2OPDS\Calibre\Types;
 
+use OCA\Calibre2OPDS\Calibre\CalibreItem;
+
 /**
  * Criteria that may be applied to books.
  */
@@ -16,4 +18,26 @@ enum CalibreBookCriteria: string {
 	case LANGUAGE = 'language';
 	case SERIES = 'series';
 	case TAG = 'tag';
+
+	/**
+	 * Reference implementing classes.
+	 *
+	 * @var array<string,class-string<CalibreItem>>
+	 */
+	private const REF_CLASSES = [
+		self::AUTHOR->value => CalibreAuthor::class,
+		self::PUBLISHER->value => CalibrePublisher::class,
+		self::LANGUAGE->value => CalibreLanguage::class,
+		self::SERIES->value => CalibreSeries::class,
+		self::TAG->value => CalibreTag::class,
+	];
+
+	/**
+	 * Get implementing class for this criterion reference.
+	 *
+	 * @return class-string<CalibreItem>|null reference implementing class, or `null` if not known.
+	 */
+	public function getDataClass(): ?string {
+		return self::REF_CLASSES[$this->value] ?? null;
+	}
 }
diff --git a/lib/Controller/OpdsController.php b/lib/Controller/OpdsController.php
@@ -7,6 +7,7 @@
 namespace OCA\Calibre2OPDS\Controller;
 
 use Exception;
+use OCA\Calibre2OPDS\Calibre\CalibreItem;
 use OCA\Calibre2OPDS\Calibre\ICalibreDB;
 use OCA\Calibre2OPDS\Calibre\Types\CalibreAuthor;
 use OCA\Calibre2OPDS\Calibre\Types\CalibreAuthorPrefix;
@@ -201,51 +202,51 @@ public function books(string $criterion = '', string $id = ''): Response {
 			$title = $this->l->t('All books');
 			$upRoute = null;
 			$upParams = [];
+			$refItem = null;
+			$refName = $id;
 			$critCase = CalibreBookCriteria::tryFrom($criterion);
+			if (!is_null($critCase) && !is_null($critClass = $critCase->getDataClass())) {
+				/**
+				 * @psalm-suppress UndefinedMethod -- this is a static method defined in subclasses
+				 * @var ?CalibreItem
+				 */
+				$refItem = $critClass::getById($lib, $id);
+				if (is_null($refItem)) {
+					return (new Response())->setStatus(Http::STATUS_NOT_FOUND);
+				}
+				if ($critCase === CalibreBookCriteria::LANGUAGE) {
+					/** @var string $refItem->code */
+					$refName = $this->settings->getLanguageName($refItem->code);
+				} else {
+					/** @var string $refItem->name */
+					$refName = $refItem->name;
+				}
+			}
 			switch ($critCase) {
-				case 'search':
-					$title = $this->l->t('All books matching: /%1$s/', [$id]);
+				case CalibreBookCriteria::SEARCH:
+					$title = $this->l->t('All books matching: /%1$s/', [$refName]);
 					break;
-				case 'author':
-					$author = CalibreAuthor::getById($lib, $id);
-					if (is_null($author)) {
-						return (new Response())->setStatus(Http::STATUS_NOT_FOUND);
-					}
-					$title = $this->l->t('All books by author: %1$s', [$author->name]);
+				case CalibreBookCriteria::AUTHOR:
+					$title = $this->l->t('All books by author: %1$s', [$refName]);
 					$upRoute = 'authors';
-					/** @var string $author->sort */
-					$upParams = [ 'prefix' => substr($author->sort, 0, intval(self::DEFAULT_PREFIX_LENGTH)) ];
+					/** @var string $refItem->sort */
+					$upParams = [ 'prefix' => substr($refItem->sort, 0, intval(self::DEFAULT_PREFIX_LENGTH)) ];
 					break;
-				case 'publisher':
-					$publisher = CalibrePublisher::getById($lib, $id);
-					if (is_null($publisher)) {
-						return (new Response())->setStatus(Http::STATUS_NOT_FOUND);
-					}
-					$title = $this->l->t('All books by publisher: %1$s', [$publisher->name]);
+				case CalibreBookCriteria::PUBLISHER:
+					$title = $this->l->t('All books by publisher: %1$s', [$refName]);
 					$upRoute = 'publishers';
 					break;
-				case 'language':
-					$language = CalibreLanguage::getById($lib, $id);
-					if (is_null($language)) {
-						return (new Response())->setStatus(Http::STATUS_NOT_FOUND);
-					}
-					/** @var string $language->code */
-					$language->setName($this->settings->getLanguageName($language->code));
-					$title = $this->l->t('All books in language: %1$s', [$language->name]);
+				case CalibreBookCriteria::LANGUAGE:
+					$title = $this->l->t('All books in language: %1$s', [$refName]);
+					$upRoute = 'languages';
 					break;
-				case 'series':
-					$series = CalibreSeries::getById($lib, $id);
-					if (is_null($series)) {
-						return (new Response())->setStatus(Http::STATUS_NOT_FOUND);
-					}
-					$title = $this->l->t('All books in series: %1$s', [$series->name]);
+				case CalibreBookCriteria::SERIES:
+					$title = $this->l->t('All books in series: %1$s', [$refName]);
+					$upRoute = 'series';
 					break;
-				case 'tag':
-					$tag = CalibreTag::getById($lib, $id);
-					if (is_null($tag)) {
-						return (new Response())->setStatus(Http::STATUS_NOT_FOUND);
-					}
-					$title = $this->l->t('All books with tag: %1$s', [$tag->name]);
+				case CalibreBookCriteria::TAG:
+					$title = $this->l->t('All books with tag: %1$s', [$refName]);
+					$upRoute = 'tags';
 					break;
 			}
 			$builder = $this->feed->createBuilder('books', $this->request->getParams(), $title, $upRoute, $upParams);

diff --git a/tests/unit/CalibreTest.php b/tests/unit/CalibreTest.php
@@ -375,4 +375,21 @@ public function testUnknownProperty(): void {
 		$this->expectExceptionMessageMatches('/Getting unknown property nonexistent_field from object of class .*/');
 		$test = $author->nonexistent_field;
 	}
+
+	public static function criteriaDataProvider(): array {
+		return [
+			[ CalibreAuthor::class ],
+			[ CalibrePublisher::class ],
+			[ CalibreLanguage::class ],
+			[ CalibreSeries::class ],
+			[ CalibreTag::class ],
+		];
+	}
+
+	#[DataProvider('criteriaDataProvider')]
+	public function testBookCriteria($critClass): void {
+		$critCase = $critClass::CRITERION;
+		$this->assertNotNull($critCase, 'Search criterion for class '.$critClass.' null check');
+		$this->assertEquals($critClass, $critCase->getDataClass(), 'Search criterion for class '.$critClass.' back reference');
+	}
 }