Skip to content

Commit

Permalink
more strict typing
Browse files Browse the repository at this point in the history
  • Loading branch information
dbu committed Sep 1, 2023
1 parent 31a782e commit 810bd6c
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 137 deletions.
2 changes: 2 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
},
"require-dev": {
"ext-json": "*",
"ext-libxml": "*",
"ext-simplexml": "*",
"psr/log": "^1 || ^2 || ^3",
"phpcr/phpcr-api-tests": "2.1.22",
"phpunit/phpunit": "^9.0",
Expand Down
8 changes: 3 additions & 5 deletions src/Jackalope/RepositoryFactoryDoctrineDBAL.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Jackalope;

use Doctrine\Common\Cache\ArrayCache;
use Doctrine\DBAL\ColumnCase;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Platforms\OraclePlatform;
Expand All @@ -13,6 +12,7 @@
use Jackalope\Transport\DoctrineDBAL\LoggingClient;
use PHPCR\ConfigurationException;
use PHPCR\RepositoryFactoryInterface;
use PHPCR\RepositoryInterface;

/**
* This factory creates repositories with the Doctrine DBAL transport.
Expand Down Expand Up @@ -77,7 +77,7 @@ class RepositoryFactoryDoctrineDBAL implements RepositoryFactoryInterface
*
* @api
*/
public function getRepository(array $parameters = null)
public function getRepository(array $parameters = null): RepositoryInterface
{
if (null === $parameters) {
throw new ConfigurationException('Jackalope-doctrine-dbal needs parameters');
Expand Down Expand Up @@ -106,9 +106,7 @@ public function getRepository(array $parameters = null)
}

$canUseCache = array_key_exists(self::JACKALOPE_DATA_CACHES, $parameters)
&& (array_key_exists('meta', $parameters[self::JACKALOPE_DATA_CACHES])
|| class_exists(ArrayCache::class)
)
&& array_key_exists('meta', $parameters[self::JACKALOPE_DATA_CACHES])
;
$transport = $canUseCache
? $factory->get(CachedClient::class, [$dbConn, $parameters[self::JACKALOPE_DATA_CACHES]])
Expand Down
4 changes: 2 additions & 2 deletions src/Jackalope/Transport/DoctrineDBAL/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ private function syncNode(?string $uuid, string $path, string $type, bool $isNew
);
} catch (\Exception $e) {
if ($e instanceof DBALException) {
if (false !== strpos($e->getMessage(), 'SQLSTATE[23')) {
if (str_contains($e->getMessage(), 'SQLSTATE[23')) {
throw new ItemExistsException('Item '.$path.' already exists in the database');
}
throw new RepositoryException(
Expand Down Expand Up @@ -1532,7 +1532,7 @@ private function cleanIdentifierCache(string $root): void
{
unset($this->nodeIdentifiers[$root]);
foreach (array_keys($this->nodeIdentifiers) as $path) {
if (0 === strpos($path, "$root/")) {
if (str_starts_with($path, "$root/")) {
unset($this->nodeIdentifiers[$path]);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Jackalope/Transport/DoctrineDBAL/LoggingClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function getConnection(): Connection
*
* Will improve error reporting at the cost of some round trips.
*/
public function setCheckLoginOnServer($bool): void
public function setCheckLoginOnServer(bool $bool): void
{
$this->transport->setCheckLoginOnServer($bool);
}
Expand Down
22 changes: 11 additions & 11 deletions src/Jackalope/Transport/DoctrineDBAL/Query/QOMWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ private function getSelectorAlias(?string $selectorName): string
} else { // Currently no aliases, use an empty string as index
$selectorAlias = '';
}
} elseif (false === strpos($selectorName, '.')) {
} elseif (!str_contains($selectorName, '.')) {
$selectorAlias = $selectorName;
} else {
$parts = explode('.', $selectorName);
$selectorAlias = reset($parts);
}

if (0 === strpos($selectorAlias, '[')) {
if (str_starts_with($selectorAlias, '[')) {
$selectorAlias = substr($selectorAlias, 1, -1);
}

Expand Down Expand Up @@ -128,10 +128,7 @@ public function walkQOMQuery(QueryObjectModel $qom): array
return [$selectors, $this->alias, $sql];
}

/**
* @return string
*/
public function getColumns(QueryObjectModel $qom)
public function getColumns(QueryObjectModel $qom): string
{
// TODO we should actually build Xpath statements for each column we actually need in the result and not fetch all 'props'
$sqlColumns = ['path', 'identifier', 'props'];
Expand Down Expand Up @@ -247,15 +244,18 @@ public function walkSelectorSource(QOM\SelectorInterface $source): string
*
* @throws \BadMethodCallException if the provided JoinCondition has no valid way of getting the right selector
*/
private function getRightJoinSelector(QOM\JoinConditionInterface $right)
private function getRightJoinSelector(QOM\JoinConditionInterface $right): string
{
if ($right instanceof QOM\ChildNodeJoinConditionInterface) {
return $right->getParentSelectorName();
} elseif ($right instanceof QOM\DescendantNodeJoinConditionInterface) {
}
if ($right instanceof QOM\DescendantNodeJoinConditionInterface) {
return $right->getAncestorSelectorName();
} elseif ($right instanceof QOM\SameNodeJoinConditionInterface || $right instanceof QOM\EquiJoinConditionInterface) {
}
if ($right instanceof QOM\SameNodeJoinConditionInterface || $right instanceof QOM\EquiJoinConditionInterface) {
return $right->getSelector2Name();
}

throw new \BadMethodCallException('Supplied join type should implement getSelector2Name() or be an instance of ChildNodeJoinConditionInterface or DescendantNodeJoinConditionInterface');
}

Expand Down Expand Up @@ -478,7 +478,7 @@ public function walkDescendantNodeConstraint(QOM\DescendantNodeInterface $constr
$ancestorPath = $constraint->getAncestorPath();
if ('/' === $ancestorPath) {
$ancestorPath = '';
} elseif ('/' === substr($ancestorPath, -1)) {
} elseif (str_ends_with($ancestorPath, '/')) {
throw new InvalidQueryException("Trailing slash in $ancestorPath");
}

Expand Down Expand Up @@ -565,7 +565,7 @@ public function walkComparisonConstraint(QOM\ComparisonInterface $constraint): s
$alias = $this->getTableAlias($selectorName);

$literal = $literalOperand->getLiteralValue();
if (false !== strpos($literal, ':')) {
if (str_contains($literal, ':')) {
$parts = explode(':', $literal);
if (!array_key_exists($parts[0], $this->namespaces)) {
throw new NamespaceException('The namespace '.$parts[0].' was not registered.');
Expand Down
48 changes: 17 additions & 31 deletions src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,17 @@
*/
class RepositorySchema extends Schema
{
/**
* @var Connection
*/
private $connection;

/**
* @var int
*/
private $maxIndexLength = -1;

/**
* @var array
*/
private $options;
private ?Connection $connection;
private ?int $maxIndexLength = -1;
private array $options;

/**
* @param array $options the options could be use to make the table names configurable
*/
public function __construct(array $options = [], Connection $connection = null)
{
$this->connection = $connection;
$schemaConfig = null === $connection ? null : $connection->getSchemaManager()->createSchemaConfig();
$schemaConfig = $connection?->getSchemaManager()->createSchemaConfig();

parent::__construct([], [], $schemaConfig);

Expand All @@ -58,7 +47,7 @@ public function __construct(array $options = [], Connection $connection = null)
/**
* Merges Jackalope schema with the given schema.
*/
public function addToSchema(Schema $schema)
public function addToSchema(Schema $schema): void
{
foreach ($this->getTables() as $table) {
$schema->_addTable($table);
Expand All @@ -69,25 +58,22 @@ public function addToSchema(Schema $schema)
}
}

protected function addNamespacesTable()
protected function addNamespacesTable(): void
{
$namespace = $this->createTable('phpcr_namespaces');
$namespace->addColumn('prefix', 'string', ['length' => $this->getMaxIndexLength()]);
$namespace->addColumn('uri', 'string');
$namespace->setPrimaryKey(['prefix']);
}

protected function addWorkspacesTable()
protected function addWorkspacesTable(): void
{
$workspace = $this->createTable('phpcr_workspaces');
$workspace->addColumn('name', 'string', ['length' => $this->getMaxIndexLength()]);
$workspace->setPrimaryKey(['name']);
}

/**
* @return Table
*/
protected function addNodesTable()
protected function addNodesTable(): Table
{
// TODO increase the size of 'path' and 'parent' but this causes issues on MySQL due to key length
$nodes = $this->createTable('phpcr_nodes');
Expand All @@ -113,15 +99,15 @@ protected function addNodesTable()
return $nodes;
}

protected function addInternalIndexTypesTable()
protected function addInternalIndexTypesTable(): void
{
$indexJcrTypes = $this->createTable('phpcr_internal_index_types');
$indexJcrTypes->addColumn('type', 'string', ['length' => $this->getMaxIndexLength()]);
$indexJcrTypes->addColumn('node_id', 'integer', ['length' => $this->getMaxIndexLength()]);
$indexJcrTypes->setPrimaryKey(['type', 'node_id']);
}

protected function addBinaryDataTable()
protected function addBinaryDataTable(): void
{
$binary = $this->createTable('phpcr_binarydata');
$binary->addColumn('id', 'integer', ['autoincrement' => true]);
Expand All @@ -134,7 +120,7 @@ protected function addBinaryDataTable()
$binary->addUniqueIndex(['node_id', 'property_name', 'workspace_name', 'idx']);
}

protected function addNodesReferencesTable(Table $nodes)
protected function addNodesReferencesTable(Table $nodes): void
{
$references = $this->createTable('phpcr_nodes_references');
$references->addColumn('source_id', 'integer');
Expand All @@ -149,7 +135,7 @@ protected function addNodesReferencesTable(Table $nodes)
}
}

protected function addNodesWeakreferencesTable(Table $nodes)
protected function addNodesWeakreferencesTable(Table $nodes): void
{
$weakreferences = $this->createTable('phpcr_nodes_weakreferences');
$weakreferences->addColumn('source_id', 'integer');
Expand All @@ -163,7 +149,7 @@ protected function addNodesWeakreferencesTable(Table $nodes)
}
}

protected function addTypeNodesTable()
protected function addTypeNodesTable(): void
{
$types = $this->createTable('phpcr_type_nodes');
$types->addColumn('node_type_id', 'integer', ['autoincrement' => true]);
Expand All @@ -178,7 +164,7 @@ protected function addTypeNodesTable()
$types->addUniqueIndex(['name']);
}

protected function addTypePropsTable()
protected function addTypePropsTable(): void
{
$propTypes = $this->createTable('phpcr_type_props');
$propTypes->addColumn('node_type_id', 'integer');
Expand All @@ -196,7 +182,7 @@ protected function addTypePropsTable()
$propTypes->setPrimaryKey(['node_type_id', 'name']);
}

protected function addTypeChildsTable()
protected function addTypeChildsTable(): void
{
$childTypes = $this->createTable('phpcr_type_childs');
$childTypes->addColumn('id', 'integer', ['autoincrement' => true]);
Expand All @@ -211,7 +197,7 @@ protected function addTypeChildsTable()
$childTypes->setPrimaryKey(['id']);
}

public function reset()
public function reset(): void
{
if (null === $this->connection) {
throw new RepositoryException('Do not use RepositorySchema::reset when not instantiated with a connection');
Expand Down Expand Up @@ -250,7 +236,7 @@ private function getMaxIndexLength($currentMaxLength = null)
return $this->maxIndexLength;
}

private function isConnectionCharsetUtf8mb4()
private function isConnectionCharsetUtf8mb4(): bool
{
if (!$this->connection) {
return false;
Expand Down
23 changes: 6 additions & 17 deletions src/Jackalope/Transport/DoctrineDBAL/Util/Xpath.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@
*/
class Xpath
{
/**
* @return string
*/
public static function escapeBackslashes($query)
public static function escapeBackslashes($query): string
{
return str_replace('\\', '\\\\', $query);

Expand All @@ -33,27 +30,21 @@ public static function escapeBackslashes($query)
* Example:
* query: Foo isn't bar
* result: concat("Foo isn", "'", "t bar")
*
* @param string $query
* @param string $enclosure
* @param bool $doubleEscapeSingleQuote
*
* @return string
*/
public static function escape($query, $enclosure = '"', $doubleEscapeSingleQuote = true)
public static function escape(string $query, string $enclosure = '"', bool $doubleEscapeSingleQuote = true): string
{
$escapeSingleQuote = $doubleEscapeSingleQuote ? '"\'%s"' : '"%s"';
$escapeDoubleQuote = $doubleEscapeSingleQuote ? "''%s''" : "'%s'";

if ((false !== strpos($query, '\''))
|| (false !== strpos($query, '"'))
if (str_contains($query, '\'')
|| str_contains($query, '"')
) {
$quotechars = ['\'', '"'];
$parts = [];
$current = '';

foreach (str_split($query) as $character) {
if (in_array($character, $quotechars)) {
if (in_array($character, $quotechars, true)) {
if ('' !== $current) {
$parts[] = $enclosure.$current.$enclosure;
}
Expand Down Expand Up @@ -90,10 +81,8 @@ public static function escape($query, $enclosure = '"', $doubleEscapeSingleQuote
/**
* Because not all concat() implementations support more then 2 arguments,
* we need this recursive function.
*
* @return string
*/
public static function concatBy2(array $parts)
public static function concatBy2(array $parts): string
{
if (2 === count($parts)) {
return sprintf('concat(%s, %s)', $parts[0], $parts[1]);
Expand Down
Loading

0 comments on commit 810bd6c

Please sign in to comment.