Merge pull request #1984 from hemberger/container-template-methods

Container template methods

src/lib/Smr/AbstractPlayer.php

@@ -1796,7 +1796,7 @@ public function getPlottedCourse(): Path|false {
 
 			if ($dbResult->hasRecord()) {
 				// get the course back
-				$this->plottedCourse = $dbResult->record()->getObject('course');
+				$this->plottedCourse = $dbResult->record()->getClass('course', Path::class);
 			} else {
 				$this->plottedCourse = false;
 			}

src/lib/Smr/Container/DiContainer.php

@@ -4,6 +4,7 @@
 
 use DI\ContainerBuilder;
 use Doctrine\DBAL\Connection;
+use Exception;
 use Smr\Database;
 use Smr\DatabaseProperties;
 use Smr\Epoch;
@@ -73,23 +74,41 @@ public static function initialize(bool $enableCompilation): void {
 
 	/**
 	 * Retrieve the managed instance of $className, or construct a new instance with all dependencies.
-	 * @param string $className The name of the class to retrieve from the container.
+	 *
+	 * @template T of object
+	 * @param class-string<T> $className The name of the class to retrieve from the container.
+	 *
 	 * @throws \DI\DependencyException
 	 * @throws \DI\NotFoundException
+	 *
+	 * @return T
 	 */
-	public static function get(string $className): mixed {
-		return self::getContainer()->get($className);
+	public static function getClass(string $className): mixed {
+		$class = self::getContainer()->get($className);
+		if (!($class instanceof $className)) {
+			throw new Exception('Expected instance of ' . $className . ' from container, got ' . gettype($class));
+		}
+		return $class;
 	}
 
 	/**
 	 * Construct a fresh instance of $className. Dependencies will be retrieved from the container if they
 	 * are already managed, and created themselves if they are not.
-	 * @param string $className The name of the class to construct.
+	 *
+	 * @template T of object
+	 * @param class-string<T> $className The name of the class to construct.
+	 *
 	 * @throws \DI\DependencyException
 	 * @throws \DI\NotFoundException
+	 *
+	 * @return T
 	 */
-	public static function make(string $className): mixed {
-		return self::getContainer()->make($className);
+	public static function makeClass(string $className): mixed {
+		$class = self::getContainer()->make($className);
+		if (!($class instanceof $className)) {
+			throw new Exception('Expected instance of ' . $className . ' from container, got ' . gettype($class));
+		}
+		return $class;
 	}
 
 	/**

src/lib/Smr/Database.php

@@ -21,7 +21,7 @@ class Database {
 	 * This is the intended way to construct this class.
 	 */
 	public static function getInstance(): self {
-		return DiContainer::get(self::class);
+		return DiContainer::getClass(self::class);
 	}
 
 	/**

src/lib/Smr/DatabaseRecord.php

@@ -68,6 +68,19 @@ public function getObject(string $name, bool $compressed = false): mixed {
 		return unserialize($object);
 	}
 
+	/**
+	 * @template T of object
+	 * @param class-string<T> $class
+	 * @return T
+	 */
+	public function getClass(string $name, string $class, bool $compressed = false): mixed {
+		$object = $this->getObject($name, $compressed);
+		if (!($object instanceof $class)) {
+			throw new Exception('Value ' . var_export($object, true) . ' is not of type ' . $class);
+		}
+		return $object;
+	}
+
 	/**
 	 * @template T of BackedEnum
 	 * @param class-string<T> $enum

src/lib/Smr/DummyShip.php

@@ -36,7 +36,7 @@ public static function getCachedDummyShip(string $dummyName): self {
 				'dummy_name' => $db->escapeString($dummyName),
 			]);
 			if ($dbResult->hasRecord()) {
-				$ship = $dbResult->record()->getObject('info');
+				$ship = $dbResult->record()->getClass('info', self::class);
 			} else {
 				$player = new DummyPlayer($dummyName);
 				$ship = new self($player);

src/lib/Smr/Epoch.php

@@ -39,7 +39,7 @@ public function getTime(): int {
 	 * and this will be the time associated with the page request.
 	 */
 	private static function getInstance(): self {
-		return DiContainer::get(self::class);
+		return DiContainer::getClass(self::class);
 	}
 
 	/**
@@ -66,7 +66,7 @@ public static function time(): int {
 	 * only be used by the CLI programs that run continuously.
 	 */
 	public static function update(): void {
-		if (DiContainer::get('NPC_SCRIPT') === false) {
+		if (DiContainer::getContainer()->get('NPC_SCRIPT') === false) {
 			throw new Exception('Only call this function from CLI programs!');
 		}
 		DiContainer::getContainer()->set(self::class, new self());

src/lib/Smr/Port.php

@@ -1219,7 +1219,7 @@ public static function getCachedPort(int $gameID, int $sectorID, int $accountID,
 
 			if ($dbResult->hasRecord()) {
 				$dbRecord = $dbResult->record();
-				self::$CACHE_CACHED_PORTS[$gameID][$sectorID][$accountID] = $dbRecord->getObject('port_info', true);
+				self::$CACHE_CACHED_PORTS[$gameID][$sectorID][$accountID] = $dbRecord->getClass('port_info', self::class, true);
 				self::$CACHE_CACHED_PORTS[$gameID][$sectorID][$accountID]->setCachedTime($dbRecord->getInt('visited'));
 			} else {
 				self::$CACHE_CACHED_PORTS[$gameID][$sectorID][$accountID] = false;

src/lib/Smr/SectorLock.php

@@ -172,7 +172,7 @@ public function release(): bool {
 	 * The first time this is called, it will populate the DI container.
 	 */
 	public static function getInstance(): self {
-		return DiContainer::get(self::class);
+		return DiContainer::getClass(self::class);
 	}
 
 	/**
@@ -183,7 +183,7 @@ public static function getInstance(): self {
 	 * only be used by the CLI programs that run continuously.
 	 */
 	public static function resetInstance(): void {
-		if (DiContainer::get('NPC_SCRIPT') === false) {
+		if (DiContainer::getContainer()->get('NPC_SCRIPT') === false) {
 			throw new Exception('Only call this function from CLI programs!');
 		}
 		// Release before resetting

src/lib/Smr/Session.php

@@ -58,7 +58,7 @@ class Session {
 	 * This is the intended way to construct this class.
 	 */
 	public static function getInstance(): self {
-		return DiContainer::get(self::class);
+		return DiContainer::getClass(self::class);
 	}
 
 	/**

src/lib/Smr/Template.php

@@ -31,7 +31,7 @@ class Template {
 	 * This is the intended way to construct this class.
 	 */
 	public static function getInstance(): self {
-		return DiContainer::get(self::class);
+		return DiContainer::getClass(self::class);
 	}
 
 	public function hasTemplateVar(string $var): bool {

test/SmrTest/BaseIntegrationSpec.php

@@ -30,7 +30,7 @@ abstract protected function tablesToTruncate(): array;
 	#[BeforeClass]
 	final public static function initializeTableRowCounts(): void {
 		if (!isset(self::$conn)) {
-			self::$conn = DiContainer::make(Connection::class);
+			self::$conn = DiContainer::makeClass(Connection::class);
 			self::$checksums = self::getChecksums();
 		}
 	}

test/SmrTest/Container/DiContainerTest.php

@@ -2,8 +2,12 @@
 
 namespace SmrTest\Container;
 
+use ArrayObject;
+use DI\Definition\StringDefinition;
+use Exception;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\RunInSeparateProcess;
+use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\TestCase;
 use Smr\Container\DiContainer;
 use Smr\DatabaseProperties;
@@ -33,30 +37,43 @@ public function test_compilation_enabled_false(): void {
 		self::assertFileDoesNotExist(self::PHPDI_COMPILED_CONTAINER_FILE);
 	}
 
-	public function test_container_get_and_make(): void {
+	public function test_getClass_and_makeClass(): void {
 		// Start with a fresh container
 		DiContainer::initialize(false);
 
 		// The first get should construct a new object
 		$class = DatabaseProperties::class;
-		$instance1 = DiContainer::get($class);
+		$instance1 = DiContainer::getClass($class);
 		self::assertInstanceOf($class, $instance1);
 
 		// Getting the same class should now give the exact same object
-		$instance2 = DiContainer::get($class);
+		$instance2 = DiContainer::getClass($class);
 		self::assertSame($instance1, $instance2);
 
 		// Using make should construct a new object
-		$instance3 = DiContainer::make($class);
+		$instance3 = DiContainer::makeClass($class);
 		self::assertNotSame($instance1, $instance3);
 		self::assertEquals($instance1, $instance3);
 	}
 
+	#[TestWith(['makeClass'])]
+	#[TestWith(['getClass'])]
+	public function test_getClass_makeClass_error(string $method): void {
+		// Set class name entry in container to something other than the
+		// instance of the class to verify that we check the type.
+		$class = ArrayObject::class;
+		DiContainer::initialize(false);
+		DiContainer::getContainer()->set($class, new StringDefinition('foo'));
+		$this->expectExceptionMessage('Expected instance of ' . $class . ' from container, got string');
+		$this->expectException(Exception::class);
+		DiContainer::$method($class);
+	}
+
 	public function test_factory_DatabaseName(): void {
 		// Start with a fresh container
 		DiContainer::initialize(false);
 		// Then make sure the 'DatabaseName' is as expected
-		$dbName = DiContainer::get('DatabaseName');
+		$dbName = DiContainer::getContainer()->get('DatabaseName');
 		self::assertSame($dbName, 'smr_live_test');
 	}
 
@@ -77,7 +94,7 @@ public function test_initialized(): void {
 		self::assertFalse(DiContainer::initialized($entry));
 
 		// Only once the entry is requested should it be initialized.
-		DiContainer::get($entry);
+		DiContainer::getContainer()->get($entry);
 		self::assertTrue(DiContainer::initialized($entry));
 	}
 

test/SmrTest/lib/DatabaseIntegrationTest.php

@@ -26,16 +26,16 @@ protected function setUp(): void {
 
 	public function test_connectionFactory(): void {
 		// Given database properties are retrieved from the container
-		$dbProperties = DiContainer::get(DatabaseProperties::class);
+		$dbProperties = DiContainer::getClass(DatabaseProperties::class);
 		// When using the factory to retrieve a connection instance
 		$conn = Database::connectionFactory($dbProperties);
 		// The the connection is successful
 		self::assertSame($dbProperties->database, $conn->getDatabase());
 	}
 
 	public function test__construct_happy_path(): void {
-		$db = DiContainer::get(Database::class);
-		self::assertNotNull($db);
+		$db = DiContainer::getClass(Database::class);
+		self::assertInstanceOf(Database::class, $db);
 	}
 
 	public function test_getInstance_always_returns_same_instance(): void {
@@ -48,17 +48,17 @@ public function test_getInstance_always_returns_same_instance(): void {
 
 	public function test_resetInstance_returns_new_instance(): void {
 		// Given an original connection instance
-		$original = DiContainer::get(Connection::class);
+		$original = DiContainer::getClass(Connection::class);
 		// And resetInstance is called
 		Database::resetInstance();
 		// And Database is usable again after reconnecting
 		Database::getInstance()->read('SELECT 1');
 		// Then new instance is not the same as the original instance
-		self::assertNotSame($original, DiContainer::get(Connection::class));
+		self::assertNotSame($original, DiContainer::getClass(Connection::class));
 	}
 
 	public function test_resetInstance_closes_connection(): void {
-		$conn = DiContainer::get(Connection::class);
+		$conn = DiContainer::getClass(Connection::class);
 		$db = Database::getInstance();
 		$db->read('SELECT 1'); // initialize the connection
 		self::assertTrue($conn->isConnected());

test/SmrTest/lib/DatabaseRecordTest.php

@@ -109,16 +109,17 @@ public function test_getFloat_with_non_float_field(mixed $value, string $error):
 
 	//------------------------------------------------------------------------
 
-	public function test_getObject(): void {
-		// Construct a record with various types of objects
-		$record = new DatabaseRecord([
-			'name' => serialize(new ArrayObject(['a' => 1, 'b' => 2])),
-			'name_compressed' => gzcompress(serialize(['c', 'd'])),
-		]);
-		// Class objects must be compared here with Equals instead of Same
-		// since the two objects will not be the same instance.
-		self::assertEquals(new ArrayObject(['a' => 1, 'b' => 2]), $record->getObject('name'));
-		self::assertSame(['c', 'd'], $record->getObject('name_compressed', compressed: true));
+	#[TestWith([true])]
+	#[TestWith([false])]
+	public function test_getObject(bool $compressed): void {
+		// Construct a record with various array objects
+		$expected = ['a' => 1, 'b' => 2];
+		$value = serialize($expected);
+		if ($compressed) {
+			$value = gzcompress($value);
+		}
+		$record = new DatabaseRecord(['name' => $value]);
+		self::assertSame($expected, $record->getObject('name', $compressed));
 	}
 
 	public function test_getNullableObject(): void {
@@ -130,6 +131,29 @@ public function test_getNullableObject(): void {
 		self::assertSame([1, 2], $record->getNullableObject('not_null'));
 	}
 
+	#[TestWith([true])]
+	#[TestWith([false])]
+	public function test_getClass(bool $compressed): void {
+		// Construct a record with a class instance
+		$expected = new ArrayObject(['a' => 1, 'b' => 2]);
+		$value = serialize($expected);
+		if ($compressed) {
+			$value = gzcompress($value);
+		}
+		$record = new DatabaseRecord(['name' => $value]);
+		// Class instances must be compared here with Equals instead of Same
+		// since the two objects will not be the same instance in memory.
+		self::assertEquals($expected, $record->getClass('name', ArrayObject::class, $compressed));
+	}
+
+	public function test_getClass_error(): void {
+		// Construct a record with a string, but then fetch as ArrayObject
+		$record = new DatabaseRecord(['name' => serialize('foo')]);
+		$this->expectException(Exception::class);
+		$this->expectExceptionMessage('Value \'foo\' is not of type ArrayObject');
+		$record->getClass('name', ArrayObject::class);
+	}
+
 	//------------------------------------------------------------------------
 
 	public function test_getIntEnum(): void {

test/SmrTest/lib/SectorLockTest.php

@@ -80,7 +80,7 @@ public function test_acquire_two_locks_in_different_requests_throws(): void {
 		// Given that we acquire a lock
 		$lock1->acquire(1, 1, 1);
 		// And we use a new SectorLock instance to simulate a separate request
-		$lock2 = DiContainer::make(SectorLock::class);
+		$lock2 = DiContainer::makeClass(SectorLock::class);
 		// Then if that instance tries to acquire a lock (even for the same
 		// sector) before the other instance releases, we throw a UserError.
 		$this->expectException(UserError::class);