Fix test interaction with smart albums (#1428)

* Added trait for interaction with smart albums
* Removed test code from productive code
* Refactored feature tests

app/SmartAlbums/BaseSmartAlbum.php

@@ -6,6 +6,8 @@
 use App\Contracts\AbstractAlbum;
 use App\Contracts\InternalLycheeException;
 use App\DTO\PhotoSortingCriterion;
+use App\Exceptions\ConfigurationKeyMissingException;
+use App\Exceptions\Internal\FrameworkException;
 use App\Exceptions\Internal\InvalidOrderDirectionException;
 use App\Exceptions\Internal\InvalidQueryModelException;
 use App\Exceptions\InvalidPropertyException;
@@ -15,6 +17,7 @@
 use App\Models\Extensions\UTCBasedTimes;
 use App\Models\Photo;
 use App\SmartAlbums\Utils\MimicModel;
+use Illuminate\Contracts\Container\BindingResolutionException;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 
@@ -44,16 +47,24 @@ abstract class BaseSmartAlbum implements AbstractAlbum
 	protected Collection $photos;
 	protected \Closure $smartPhotoCondition;
 
+	/**
+	 * @throws ConfigurationKeyMissingException
+	 * @throws FrameworkException
+	 */
 	protected function __construct(string $id, string $title, bool $isPublic, \Closure $smartCondition)
 	{
-		$this->photoAuthorisationProvider = resolve(PhotoAuthorisationProvider::class);
-		$this->id = $id;
-		$this->title = $title;
-		$this->isPublic = $isPublic;
-		$this->isDownloadable = Configs::getValueAsBool('downloadable');
-		$this->isShareButtonVisible = Configs::getValueAsBool('share_button_visible');
-		$this->thumb = null;
-		$this->smartPhotoCondition = $smartCondition;
+		try {
+			$this->photoAuthorisationProvider = resolve(PhotoAuthorisationProvider::class);
+			$this->id = $id;
+			$this->title = $title;
+			$this->isPublic = $isPublic;
+			$this->isDownloadable = Configs::getValueAsBool('downloadable');
+			$this->isShareButtonVisible = Configs::getValueAsBool('share_button_visible');
+			$this->thumb = null;
+			$this->smartPhotoCondition = $smartCondition;
+		} catch (BindingResolutionException $e) {
+			throw new FrameworkException('Laravel\'s service container', $e);
+		}
 	}
 
 	/**

app/SmartAlbums/PublicAlbum.php

@@ -2,6 +2,8 @@
 
 namespace App\SmartAlbums;
 
+use App\Exceptions\ConfigurationKeyMissingException;
+use App\Exceptions\Internal\FrameworkException;
 use Illuminate\Database\Eloquent\Builder;
 
 /**
@@ -36,6 +38,9 @@ class PublicAlbum extends BaseSmartAlbum
 	 * **This is intended behaviour!**
 	 * See description of the whole class {@link PublicAlbum} for an
 	 * explanation.
+	 *
+	 * @throws ConfigurationKeyMissingException
+	 * @throws FrameworkException
 	 */
 	protected function __construct()
 	{
@@ -49,17 +54,6 @@ protected function __construct()
 
 	public static function getInstance(): self
 	{
-		self::$instance ??= new self();
-
-		// The following two lines are only needed due to testing.
-		// The same instance of this class is used for all tests, because
-		// the singleton stays alive during tests.
-		// This implies that the relation of photos is never reloaded
-		// but remains constant during all tests (it equals the empty set)
-		// and the tests fail.
-		unset(self::$instance->photos);
-		unset(self::$instance->thumb);
-
-		return self::$instance;
+		return self::$instance ??= new self();
 	}
 }

app/SmartAlbums/RecentAlbum.php

@@ -2,6 +2,8 @@
 
 namespace App\SmartAlbums;
 
+use App\Exceptions\ConfigurationKeyMissingException;
+use App\Exceptions\Internal\FrameworkException;
 use App\Models\Configs;
 use Carbon\Exceptions\InvalidFormatException;
 use Carbon\Exceptions\InvalidTimeZoneException;
@@ -17,6 +19,8 @@ class RecentAlbum extends BaseSmartAlbum
 	/**
 	 * @throws InvalidFormatException
 	 * @throws InvalidTimeZoneException
+	 * @throws ConfigurationKeyMissingException
+	 * @throws FrameworkException
 	 */
 	protected function __construct()
 	{
@@ -36,17 +40,6 @@ function (Builder $query) use ($strRecent) {
 
 	public static function getInstance(): self
 	{
-		self::$instance ??= new self();
-
-		// The following two lines are only needed due to testing.
-		// The same instance of this class is used for all tests, because
-		// the singleton stays alive during tests.
-		// This implies that the relation of photos is never be reloaded
-		// but remains constant during all tests (it equals the empty set)
-		// and the tests fails.
-		unset(self::$instance->photos);
-		unset(self::$instance->thumb);
-
-		return self::$instance;
+		return self::$instance ??= new self();
 	}
 }

app/SmartAlbums/StarredAlbum.php

@@ -2,6 +2,8 @@
 
 namespace App\SmartAlbums;
 
+use App\Exceptions\ConfigurationKeyMissingException;
+use App\Exceptions\Internal\FrameworkException;
 use App\Models\Configs;
 use Illuminate\Database\Eloquent\Builder;
 
@@ -11,6 +13,10 @@ class StarredAlbum extends BaseSmartAlbum
 	public const ID = 'starred';
 	public const TITLE = 'Starred';
 
+	/**
+	 * @throws ConfigurationKeyMissingException
+	 * @throws FrameworkException
+	 */
 	protected function __construct()
 	{
 		parent::__construct(
@@ -23,17 +29,6 @@ protected function __construct()
 
 	public static function getInstance(): self
 	{
-		self::$instance ??= new self();
-
-		// The following two lines are only needed due to testing.
-		// The same instance of this class is used for all tests, because
-		// the singleton stays alive during tests.
-		// This implies that the relation of photos is never be reloaded
-		// but remains constant during all tests (it equals the empty set)
-		// and the tests fails.
-		unset(self::$instance->photos);
-		unset(self::$instance->thumb);
-
-		return self::$instance;
+		return self::$instance ??= new self();
 	}
 }

app/SmartAlbums/UnsortedAlbum.php

@@ -2,6 +2,8 @@
 
 namespace App\SmartAlbums;
 
+use App\Exceptions\ConfigurationKeyMissingException;
+use App\Exceptions\Internal\FrameworkException;
 use Illuminate\Database\Eloquent\Builder;
 
 class UnsortedAlbum extends BaseSmartAlbum
@@ -10,6 +12,10 @@ class UnsortedAlbum extends BaseSmartAlbum
 	public const ID = 'unsorted';
 	public const TITLE = 'Unsorted';
 
+	/**
+	 * @throws ConfigurationKeyMissingException
+	 * @throws FrameworkException
+	 */
 	public function __construct()
 	{
 		parent::__construct(
@@ -22,17 +28,6 @@ public function __construct()
 
 	public static function getInstance(): self
 	{
-		self::$instance ??= new self();
-
-		// The following two lines are only needed due to testing.
-		// The same instance of this class is used for all tests, because
-		// the singleton stays alive during tests.
-		// This implies that the relation of photos is never be reloaded
-		// but remains constant during all tests (it equals the empty set)
-		// and the tests fails.
-		unset(self::$instance->photos);
-		unset(self::$instance->thumb);
-
-		return self::$instance;
+		return self::$instance ??= new self();
 	}
 }

tests/Feature/AlbumTest.php

@@ -13,13 +13,20 @@
 namespace Tests\Feature;
 
 use App\Facades\AccessControl;
+use App\SmartAlbums\PublicAlbum;
+use App\SmartAlbums\RecentAlbum;
+use App\SmartAlbums\StarredAlbum;
+use App\SmartAlbums\UnsortedAlbum;
 use Illuminate\Support\Facades\DB;
 use Tests\Feature\Lib\AlbumsUnitTest;
 use Tests\Feature\Lib\RootAlbumUnitTest;
+use Tests\Feature\Traits\InteractWithSmartAlbums;
 use Tests\TestCase;
 
 class AlbumTest extends TestCase
 {
+	use InteractWithSmartAlbums;
+
 	protected AlbumsUnitTest $albums_tests;
 	protected RootAlbumUnitTest $root_album_tests;
 
@@ -37,12 +44,13 @@ public function setUp(): void
 	 */
 	public function testAddNotLogged(): void
 	{
+		$this->clearCachedSmartAlbums();
 		$this->albums_tests->add(null, 'test_album', 401);
 
-		$this->albums_tests->get('recent', 401);
-		$this->albums_tests->get('starred', 401);
-		$this->albums_tests->get('public', 401);
-		$this->albums_tests->get('unsorted', 401);
+		$this->albums_tests->get(RecentAlbum::ID, 401);
+		$this->albums_tests->get(StarredAlbum::ID, 401);
+		$this->albums_tests->get(PublicAlbum::ID, 401);
+		$this->albums_tests->get(UnsortedAlbum::ID, 401);
 
 		// Ensure that we get proper 404 (not found) response for a
 		// non-existing album, not a false 403 (forbidden) response
@@ -52,11 +60,12 @@ public function testAddNotLogged(): void
 	public function testAddReadLogged(): void
 	{
 		AccessControl::log_as_id(0);
+		$this->clearCachedSmartAlbums();
 
-		$this->albums_tests->get('recent');
-		$this->albums_tests->get('starred');
-		$this->albums_tests->get('public');
-		$this->albums_tests->get('unsorted');
+		$this->albums_tests->get(RecentAlbum::ID);
+		$this->albums_tests->get(StarredAlbum::ID);
+		$this->albums_tests->get(PublicAlbum::ID);
+		$this->albums_tests->get(UnsortedAlbum::ID);
 
 		$albumID1 = $this->albums_tests->add(null, 'test_album')->offsetGet('id');
 		$albumID2 = $this->albums_tests->add(null, 'test_album2')->offsetGet('id');

tests/Feature/GeoDataTest.php

@@ -14,15 +14,18 @@
 
 use App\Facades\AccessControl;
 use App\Models\Configs;
+use App\SmartAlbums\UnsortedAlbum;
 use Carbon\Carbon;
 use Tests\Feature\Lib\AlbumsUnitTest;
 use Tests\Feature\Lib\PhotosUnitTest;
+use Tests\Feature\Traits\InteractWithSmartAlbums;
 use Tests\Feature\Traits\RequiresEmptyPhotos;
 use Tests\TestCase;
 
 class GeoDataTest extends TestCase
 {
 	use RequiresEmptyPhotos;
+	use InteractWithSmartAlbums;
 
 	protected PhotosUnitTest $photos_tests;
 	protected AlbumsUnitTest $albums_tests;
@@ -59,7 +62,7 @@ public function testGeo(): void
 			);
 			$photoID = $photoResponse->offsetGet('id');
 
-			$this->photos_tests->see_in_unsorted($photoID);
+			$this->albums_tests->get(UnsortedAlbum::ID, 200, $photoID);
 			/*
 			 * Check some Exif data
 			 * The metadata extractor is unable to extract an explicit timezone
@@ -113,7 +116,8 @@ public function testGeo(): void
 			$albumResponse = $this->albums_tests->add(null, 'test_mongolia');
 			$albumID = $albumResponse->offsetGet('id');
 			$this->photos_tests->set_album($albumID, [$photoID]);
-			$this->photos_tests->dont_see_in_unsorted($photoID);
+			$this->clearCachedSmartAlbums();
+			$this->albums_tests->get(UnsortedAlbum::ID, 200, null, $photoID);
 			$albumResponse = $this->albums_tests->get($albumID);
 			$album = static::convertJsonToObject($albumResponse);
 			static::assertCount(1, $album->photos);

tests/Feature/Lib/AlbumsUnitTest.php

@@ -110,13 +110,15 @@ public function move(
 	 * @param string      $id
 	 * @param int         $expectedStatusCode
 	 * @param string|null $assertSee
+	 * @param string|null $assertDontSee
 	 *
 	 * @return TestResponse
 	 */
 	public function get(
 		string $id,
 		int $expectedStatusCode = 200,
-		?string $assertSee = null
+		?string $assertSee = null,
+		?string $assertDontSee = null
 	): TestResponse {
 		$response = $this->testCase->postJson(
 			'/api/Album::get',
@@ -126,6 +128,9 @@ public function get(
 		if ($assertSee) {
 			$response->assertSee($assertSee, false);
 		}
+		if ($assertDontSee) {
+			$response->assertDontSee($assertDontSee, false);
+		}
 
 		return $response;
 	}