Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added check for valid SQL before running fetchAll. #12

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 50 additions & 8 deletions src/Server/Resource/DoctrineResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@
use Doctrine\ODM\MongoDB\Query\Builder as MongoDBQueryBuilder;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\NoResultException;
use Doctrine\ORM\Query as ORMQuery;
use Doctrine\ORM\Query\QueryException;
use DoctrineModule\Persistence\ObjectManagerAwareInterface;
use DoctrineModule\Stdlib\Hydrator;
use Laminas\ApiTools\ApiProblem\ApiProblem;
use Laminas\ApiTools\ApiProblem\ApiProblemResponse;
use Laminas\ApiTools\ApiProblem\Exception\DomainException;
use Laminas\ApiTools\Doctrine\Server\Event\DoctrineResourceEvent;
use Laminas\ApiTools\Doctrine\Server\Exception\InvalidArgumentException;
use Laminas\ApiTools\Doctrine\Server\Paginator\Adapter\DoctrineOrmAdapter
use Laminas\ApiTools\Doctrine\Server\Query\CreateFilter\QueryCreateFilterInterface;
use Laminas\ApiTools\Doctrine\Server\Query\Provider\QueryProviderInterface;
use Laminas\ApiTools\Rest\AbstractResourceListener;
Expand Down Expand Up @@ -94,12 +99,18 @@ class DoctrineResource extends AbstractResourceListener implements
*/
private $entityFactory;

/**
* @var bool $displayExceptions
*/
private $displayExceptions;

/**
* @param InstantiatorInterface|null $entityFactory
*/
public function __construct(InstantiatorInterface $entityFactory = null)
public function __construct(InstantiatorInterface $entityFactory = null, $displayExceptions)
{
$this->entityFactory = $entityFactory;
$this->displayExceptions = $displayExceptions;
}

/**
Expand Down Expand Up @@ -564,13 +575,44 @@ function (EventInterface $e) use ($queryProvider, $entityClass, $data) {
$halCollection = $e->getParam('collection');
$collection = $halCollection->getCollection();

$collection->setItemCountPerPage($halCollection->getPageSize());
$collection->setCurrentPageNumber($halCollection->getPage());

$halCollection->setCollectionRouteOptions([
'query' => $e->getTarget()->getRequest()->getQuery()->toArray(),
]);
}
/**
* Validate the query. laminas-api-tools-doctrine-querybuilder
* can form invalid SQL.
* This method is used to catch QueryExceptions then format a
* DomainException them so inner-working of the ORM are not
* divulged to the user.
*
* Doctrine by default gives verbose exception messages but
* they can tell too much. The api-tools-doctrine-querybuilder
* allows the user to name fields and if a field name specified
* by the user does not exist an exception will be thrown.
* This is the primary reason for throwing a composed
* DomainException here.
*
* Calling getSql forces the Query to parse()
* ODM fails gracefully and does not need a catch.
*/
if ($collection->getAdapter() instanceof DoctrineOrmAdapter) {
try {
$collection->getAdapter()->getQuery()->getSQL();
} catch (QueryException $e) {

$exception = new DomainException('Invalid query.', $e->getCode());
if ($this->displayExceptions) {
$exception->setAdditionalDetails([$e->getMessage()]);
}

throw $exception;
}

$collection->setItemCountPerPage($halCollection->getPageSize());
$collection->setCurrentPageNumber($halCollection->getPage());

$halCollection->setCollectionRouteOptions([
'query' => $e->getTarget()->getRequest()->getQuery()->toArray(),
]);
}
}, 100
);

return $collection;
Expand Down
8 changes: 6 additions & 2 deletions src/Server/Resource/DoctrineResourceFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,17 @@ public function __invoke(ContainerInterface $container, $requestedName, array $o
? $container->get($doctrineConnectedConfig['entity_factory'])
: null;

$displayExceptions = $config['view_manager']['display_exceptions'] ?? false;
$hydrator = $this->loadHydrator($container, $doctrineConnectedConfig, $doctrineHydratorConfig);
$queryProviders = $this->loadQueryProviders($container, $doctrineConnectedConfig, $objectManager);
$queryCreateFilter = $this->loadQueryCreateFilter($container, $doctrineConnectedConfig, $objectManager);
$configuredListeners = $this->loadConfiguredListeners($container, $doctrineConnectedConfig);

/** @var DoctrineResource $listener */
$listener = new $resourceClass($entityFactory);
/**
* @var DoctrineResource $listener
* @var bool $displayExceptions
*/
$listener = new $resourceClass($entityFactory, $displayExceptions);
$listener->setSharedEventManager($container->get('Application')->getEventManager()->getSharedManager());
$listener->setObjectManager($objectManager);
$listener->setHydrator($hydrator);
Expand Down
36 changes: 36 additions & 0 deletions test/src/Server/ORM/CRUD/CRUDTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
use Laminas\ApiTools\Doctrine\DoctrineResource;
use Laminas\ApiTools\Doctrine\Server\Event\DoctrineResourceEvent;
use Laminas\ApiTools\Rest\ResourceEvent;
use Laminas\ApiTools\Rest\RestController;
use Laminas\EventManager\EventInterface;
use Laminas\Filter\FilterChain;
use Laminas\Http\Request;
use Laminas\ServiceManager\ServiceManager;
Expand Down Expand Up @@ -247,6 +249,40 @@ public function testCreateWithListenerThatReturnsApiProblem($method, $message)
);
}

public function testFetchAllInvalidQueryThrows500ViaDomainException()
{
$product = $this->createProduct();
$product = $this->createProduct();
$product = $this->createProduct();

$this->getRequest()->getHeaders()->addHeaderLine('Accept', 'application/json');
$this->getRequest()->setMethod(Request::METHOD_GET);

$sharedEventManager = ($this->getApplication()->getEventManager()->getSharedManager())
->attach(
RestController::class,
'getList.post',
function (EventInterface $e) {
/** @var \Laminas\ApiTools\Hal\Collection $halCollection */
$halCollection = $e->getParam('collection');
$collection = $halCollection->getCollection();

// Set an invalid query to trigger API Problem Exception
$query = $collection->getAdapter()->getQuery()
->setDql('SELECT test.invalid FROM test');
;
}, 100000
);


$this->dispatch('/test/rest/product');
$body = json_decode($this->getResponse()->getBody(), true);

$this->assertEquals(500, $this->getResponse()->getStatusCode());
$this->assertEquals('Invalid query.', $body['detail']);
$this->assertEquals('Internal Server Error', $body['title']);
}

public function testFetchByCustomIdField()
{
$product = $this->createProduct();
Expand Down