From 4d7edc527adb16cb2f176f35e3c4d131eba8dd18 Mon Sep 17 00:00:00 2001 From: Justin Ruiter Date: Thu, 4 Apr 2024 15:15:46 +0200 Subject: [PATCH] Implemented the second part of the smaller suggestions --- src/Datasource/Connection.php | 10 ++-- src/Datasource/Marshaller.php | 24 ++++---- src/Datasource/Query.php | 60 ++++++++++--------- src/Model/Endpoint.php | 41 +++++++------ src/Model/EndpointLocator.php | 17 ++++-- src/{Plugin.php => WebservicePlugin.php} | 2 +- tests/TestCase/AbstractDriverTest.php | 10 ++-- tests/TestCase/Webservice/WebserviceTest.php | 6 +- tests/bootstrap.php | 4 +- .../Driver/{TestDriver.php => Test.php} | 2 +- 10 files changed, 96 insertions(+), 80 deletions(-) rename src/{Plugin.php => WebservicePlugin.php} (94%) rename tests/test_app/src/Webservice/Driver/{TestDriver.php => Test.php} (93%) diff --git a/src/Datasource/Connection.php b/src/Datasource/Connection.php index 39b492a..63a161b 100644 --- a/src/Datasource/Connection.php +++ b/src/Datasource/Connection.php @@ -8,7 +8,6 @@ use Muffin\Webservice\Datasource\Exception\MissingConnectionException; use Muffin\Webservice\Webservice\Driver\AbstractDriver; use Muffin\Webservice\Webservice\Exception\MissingDriverException; -use Muffin\Webservice\Webservice\Exception\UnexpectedDriverException; use Psr\SimpleCache\CacheInterface; /** @@ -50,9 +49,10 @@ public function __construct(array $config) $tempDriver = new $driver($config); - if (!($tempDriver instanceof AbstractDriver)) { - throw new UnexpectedDriverException(['driver' => $driver]); - } + assert( + $tempDriver instanceof AbstractDriver, + '`$config[\'driver\']` must be an instance of `' . AbstractDriver::class . '`.' + ); $this->_driver = $tempDriver; } @@ -121,7 +121,7 @@ protected function _normalizeConfig(array $config): array throw new MissingConnectionException(['name' => $config['name']]); } - $config['driver'] = App::className($config['service'], 'Webservice/Driver', 'Driver'); + $config['driver'] = App::className($config['service'], 'Webservice/Driver'); if (!$config['driver']) { throw new MissingDriverException(['driver' => $config['driver']]); } diff --git a/src/Datasource/Marshaller.php b/src/Datasource/Marshaller.php index 5045ab2..becb2fd 100644 --- a/src/Datasource/Marshaller.php +++ b/src/Datasource/Marshaller.php @@ -109,21 +109,25 @@ protected function _validate(array $data, array $options, bool $isNew): array if (!$options['validate']) { return []; } + + $validator = null; if ($options['validate'] === true) { - $options['validate'] = $this->_endpoint->getValidator('default'); + $validator = $this->_endpoint->getValidator('default'); + } elseif (is_string($options['validate'])) { + $validator = $this->_endpoint->getValidator($options['validate']); + } else { + /** @var \Cake\Validation\Validator $validator */ + $validator = $options['validator']; } - if (is_string($options['validate'])) { - $options['validate'] = $this->_endpoint->getValidator($options['validate']); - } - if (!is_object($options['validate'])) { - throw new RuntimeException( - sprintf('validate must be a boolean, a string or an object. Got %s.', gettype($options['validate'])) - ); + if (!is_callable([$validator, 'validate'])) { + throw new RuntimeException(sprintf( + '"validate" must be a boolean, a string or an object with method "errors()". Got %s instead.', + gettype($options['validate']) + )); } - /* @phpstan-ignore-next-line Magic method */ - return $options['validate']->validate($data, $isNew); + return $validator->validate($data, $isNew); } /** diff --git a/src/Datasource/Query.php b/src/Datasource/Query.php index f688e8f..88918a5 100644 --- a/src/Datasource/Query.php +++ b/src/Datasource/Query.php @@ -380,9 +380,11 @@ public function toArray(): array */ public function setRepository(RepositoryInterface $repository): Query { - if ($repository instanceof Endpoint) { - $this->_endpoint = $repository; - } + assert( + $repository instanceof Endpoint, + '`$repository` must be an instance of `' . Endpoint::class . '`.' + ); + $this->_endpoint = $repository; return $this; } @@ -391,9 +393,9 @@ public function setRepository(RepositoryInterface $repository): Query * Returns the default repository object that will be used by this query, * that is, the table that will appear in the from clause. * - * @return \Cake\Datasource\RepositoryInterface + * @return \Muffin\Webservice\Model\Endpoint */ - public function getRepository(): RepositoryInterface + public function getRepository(): Endpoint { return $this->_endpoint; } @@ -512,11 +514,33 @@ public function getWebservice(): WebserviceInterface return $this->_webservice; } + /** + * Apply custom finds to against an existing query object. + * + * Allows custom find methods to be combined and applied to each other. + * + * ``` + * $repository->find('all')->find('recent'); + * ``` + * + * The above is an example of stacking multiple finder methods onto + * a single query. + * + * @param string $finder The finder method to use. + * @param mixed ...$args Arguments that match up to finder-specific parameters + * @return static Returns a modified query. + * @psalm-suppress MoreSpecificReturnType Couldn't get it to work with the interface and has no impact **/ + public function find(string $finder, mixed ...$args): static + { + /** @psalm-suppress LessSpecificReturnStatement Couldn't get it to work with the interface and has no impact **/ + return $this->_endpoint->callFinder($finder, $this, $args); /* @phpstan-ignore-line */ + } + /** * Get the first result from the executing query or raise an exception. * * @return mixed The first result from the ResultSet. - * @throws \Cake\Datasource\Exception\RecordNotFoundException When there is no first record. + * @throws \Cake\Datasource\Exception\RecordNotFoundException|\Exception When there is no first record. */ public function firstOrFail(): mixed { @@ -668,7 +692,7 @@ public function set(Closure|array|string $fields): Query /** * @inheritDoc */ - public function offset(?int $offset): Query|QueryInterface + public function offset(?int $offset): QueryInterface { $this->_parts['offset'] = $offset; @@ -1093,26 +1117,4 @@ public function formatResults(?Closure $formatter = null, int|bool $mode = self: return $this; } - - /** - * Apply custom finds to against an existing query object. - * - * Allows custom find methods to be combined and applied to each other. - * - * ``` - * $repository->find('all')->find('recent'); - * ``` - * - * The above is an example of stacking multiple finder methods onto - * a single query. - * - * @param string $finder The finder method to use. - * @param mixed ...$args Arguments that match up to finder-specific parameters - * @return static Returns a modified query. - * @psalm-suppress MoreSpecificReturnType Couldn't get it to work with the interface and has no impact **/ - public function find(string $finder, mixed ...$args): static - { - /** @psalm-suppress LessSpecificReturnStatement Couldn't get it to work with the interface and has no impact **/ - return $this->_endpoint->callFinder($finder, $this, $args); /* @phpstan-ignore-line */ - } } diff --git a/src/Model/Endpoint.php b/src/Model/Endpoint.php index 1ebe23f..94841c5 100644 --- a/src/Model/Endpoint.php +++ b/src/Model/Endpoint.php @@ -149,6 +149,7 @@ class Endpoint implements RepositoryInterface, EventListenerInterface, EventDisp * passed to it. * * @param array $config List of options for this endpoint + * @throws \Exception */ final public function __construct(array $config = []) { @@ -331,10 +332,15 @@ public function setConnection(Connection $connection): Endpoint /** * Returns the connection driver. * - * @return \Muffin\Webservice\Datasource\Connection|null + * @return \Muffin\Webservice\Datasource\Connection */ - public function getConnection(): ?Connection + public function getConnection(): Connection { + assert( + $this->_connection !== null, + 'Connection is null, there is no connection to return.' + ); + return $this->_connection; } @@ -371,7 +377,7 @@ public function setSchema(Schema|array $schema): Endpoint public function getSchema(): ?Schema { if ($this->_schema === null) { - $this->_schema = $this->getWebservice()?->describe($this->getName()); + $this->_schema = $this->getWebservice()->describe($this->getName()); } return $this->_schema; @@ -385,6 +391,7 @@ public function getSchema(): ?Schema * * @param string $field The field to check for. * @return bool True if the field exists, false if it does not. + * @throws \Exception */ public function hasField(string $field): bool { @@ -548,10 +555,8 @@ public function getInflectionMethod(): string public function setWebservice(string $alias, WebserviceInterface $webservice): Endpoint { $connection = $this->getConnection(); - if ($connection !== null) { - $connection->setWebservice($alias, $webservice); - $this->_webservice = $connection->getWebservice($alias); - } + $connection->setWebservice($alias, $webservice); + $this->_webservice = $connection->getWebservice($alias); return $this; } @@ -559,12 +564,14 @@ public function setWebservice(string $alias, WebserviceInterface $webservice): E /** * Get this endpoints associated webservice * - * @return \Muffin\Webservice\Webservice\WebserviceInterface|null + * @return \Muffin\Webservice\Webservice\WebserviceInterface + * @throws \Exception */ - public function getWebservice(): ?WebserviceInterface + public function getWebservice(): WebserviceInterface { + // If no webservice is found, get it from the connection if ($this->_webservice === null) { - $this->_webservice = $this->getConnection()?->getWebservice($this->getName()); + $this->_webservice = $this->getConnection()->getWebservice($this->getName()); } return $this->_webservice; @@ -582,6 +589,7 @@ public function getWebservice(): ?WebserviceInterface * @param string $type the type of query to perform * @param mixed ...$args Arguments that match up to finder-specific parameters * @return \Cake\Datasource\QueryInterface + * @throws \Exception */ public function find(string $type = 'all', mixed ...$args): QueryInterface { @@ -748,6 +756,7 @@ protected function _setFieldMatchers(array $options, array $keys): array * @param mixed ...$args Additional arguments for configuring things like caching. * @psalm-suppress InvalidReturnType For backwards compatibility. This function can also return array * @return \Cake\Datasource\EntityInterface + * @throws \Exception * @see \Cake\Datasource\RepositoryInterface::find() */ public function get( @@ -787,7 +796,7 @@ public function get( if ($cacheKey !== null) { $cacheKey = sprintf( 'get:%s.%s%s', - $this->getConnection()?->configName() ?? 'None', + $this->getConnection()->configName(), $this->getName(), json_encode($primaryKey) ); @@ -853,9 +862,6 @@ public function findOrCreate(mixed $search, ?callable $callback = null): EntityI public function query(): Query { $webservice = $this->getWebservice(); - if ($webservice === null) { - throw new Exception('Webservice not initialized, cannot create query'); - } return new Query($webservice, $this); } @@ -871,6 +877,7 @@ public function query(): Query * @param \Closure|array|string|null $conditions Conditions to be used, accepts anything Query::where() can take. * @return int Count Returns the affected rows. * @psalm-suppress MoreSpecificImplementedParamType + * @throws \Exception */ public function updateAll(Closure|array|string $fields, Closure|array|string|null $conditions): int { @@ -1319,11 +1326,7 @@ public function buildRules(RulesChecker $rules): RulesChecker */ public function __debugInfo(): array { - $connectionName = ''; - if ($this->getConnection() !== null) { - /** @psalm-suppress PossiblyNullReference getConnection cannot be null, as checked before entering this scope **/ - $connectionName = $this->getConnection()->configName(); - } + $connectionName = $this->getConnection()->configName(); return [ 'registryAlias' => $this->getRegistryAlias(), diff --git a/src/Model/EndpointLocator.php b/src/Model/EndpointLocator.php index 2d6d470..9ac8c92 100644 --- a/src/Model/EndpointLocator.php +++ b/src/Model/EndpointLocator.php @@ -36,12 +36,19 @@ public function set(string $alias, RepositoryInterface $repository): Endpoint * * @param string $alias The alias name you want to get. * @param array $options The options you want to build the endpoint with. - * @return \Cake\Datasource\RepositoryInterface + * @return \Muffin\Webservice\Model\Endpoint * @throws \RuntimeException If the registry alias is already in use. */ - public function get(string $alias, array $options = []): RepositoryInterface + public function get(string $alias, array $options = []): Endpoint { - return parent::get($alias, $options); + $parentRes = parent::get($alias, $options); + + assert( + $parentRes instanceof Endpoint, + 'The repository found is not of type Endpoint, but a different type implementing RepositoryInterface' + ); + + return $parentRes; } /** @@ -49,9 +56,9 @@ public function get(string $alias, array $options = []): RepositoryInterface * * @param string $alias Endpoint alias. * @param array $options The alias to check for. - * @return \Cake\Datasource\RepositoryInterface + * @return \Muffin\Webservice\Model\Endpoint */ - protected function createInstance(string $alias, array $options): RepositoryInterface + protected function createInstance(string $alias, array $options): Endpoint { [, $classAlias] = pluginSplit($alias); $options = ['alias' => $classAlias] + $options; diff --git a/src/Plugin.php b/src/WebservicePlugin.php similarity index 94% rename from src/Plugin.php rename to src/WebservicePlugin.php index 687ee86..4200825 100644 --- a/src/Plugin.php +++ b/src/WebservicePlugin.php @@ -8,7 +8,7 @@ use Cake\Datasource\FactoryLocator; use Muffin\Webservice\Model\EndpointLocator; -class Plugin extends BasePlugin +class WebservicePlugin extends BasePlugin { /** * Disable routes hook. diff --git a/tests/TestCase/AbstractDriverTest.php b/tests/TestCase/AbstractDriverTest.php index a0d7594..43ded93 100644 --- a/tests/TestCase/AbstractDriverTest.php +++ b/tests/TestCase/AbstractDriverTest.php @@ -7,7 +7,7 @@ use Cake\TestSuite\TestCase; use SomeVendor\SomePlugin\Webservice\Driver\SomePlugin; use StdClass; -use TestApp\Webservice\Driver\TestDriver; +use TestApp\Webservice\Driver\Test; use TestApp\Webservice\Logger; use TestApp\Webservice\TestWebservice; use TestPlugin\Webservice\Driver\TestPlugin; @@ -40,7 +40,7 @@ public function testSetClient() { $client = new StdClass(); - $driver = new TestDriver(); + $driver = new Test(); $driver->setClient($client); $this->assertSame($client, $driver->getClient()); @@ -48,7 +48,7 @@ public function testSetClient() public function testEnableQueryLogging() { - $driver = new TestDriver(); + $driver = new Test(); $driver->enableQueryLogging(); $this->assertTrue($driver->isQueryLoggingEnabled()); @@ -56,7 +56,7 @@ public function testEnableQueryLogging() public function testDisableQueryLogging() { - $driver = new TestDriver(); + $driver = new Test(); $driver->disableQueryLogging(); $this->assertFalse($driver->isQueryLoggingEnabled()); @@ -74,7 +74,7 @@ public function testDebugInfo() 'webservices' => ['example'], ]; - $driver = new TestDriver(); + $driver = new Test(); $driver->setLogger($logger); $driver ->setClient($client) diff --git a/tests/TestCase/Webservice/WebserviceTest.php b/tests/TestCase/Webservice/WebserviceTest.php index 8204580..4ab6c9b 100644 --- a/tests/TestCase/Webservice/WebserviceTest.php +++ b/tests/TestCase/Webservice/WebserviceTest.php @@ -9,7 +9,7 @@ use Muffin\Webservice\Model\Exception\MissingEndpointSchemaException; use Muffin\Webservice\Webservice\Exception\UnimplementedWebserviceMethodException; use Muffin\Webservice\Webservice\Webservice; -use TestApp\Webservice\Driver\TestDriver; +use TestApp\Webservice\Driver\Test; use TestApp\Webservice\TestWebservice; class WebserviceTest extends TestCase @@ -27,7 +27,7 @@ public function setUp(): void parent::setUp(); $this->webservice = new TestWebservice([ - 'driver' => new TestDriver([]), + 'driver' => new Test([]), ]); } @@ -43,7 +43,7 @@ public function tearDown(): void public function testConstructor() { - $testDriver = new TestDriver([]); + $testDriver = new Test([]); $webservice = new TestWebservice([ 'driver' => $testDriver, diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 430d3d3..1e2a3fa 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -18,7 +18,7 @@ use Cake\Datasource\ConnectionManager; use Cake\Log\Log; use Muffin\Webservice\Datasource\Connection; -use TestApp\Webservice\Driver\TestDriver; +use TestApp\Webservice\Driver\Test; require_once 'vendor/autoload.php'; @@ -88,7 +88,7 @@ ConnectionManager::setConfig('test', [ 'className' => Connection::class, - 'driver' => TestDriver::class, + 'driver' => Test::class, ] + ConnectionManager::parseDsn(getenv('DB_DSN'))); Log::setConfig([ diff --git a/tests/test_app/src/Webservice/Driver/TestDriver.php b/tests/test_app/src/Webservice/Driver/Test.php similarity index 93% rename from tests/test_app/src/Webservice/Driver/TestDriver.php rename to tests/test_app/src/Webservice/Driver/Test.php index 5699610..f451e6f 100644 --- a/tests/test_app/src/Webservice/Driver/TestDriver.php +++ b/tests/test_app/src/Webservice/Driver/Test.php @@ -7,7 +7,7 @@ use Muffin\Webservice\Webservice\WebserviceInterface; use TestApp\Webservice\EndpointTestWebservice; -class TestDriver extends AbstractDriver +class Test extends AbstractDriver { /** * Initialize is used to easily extend the constructor.