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

some improvements #3

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/vendor
/.idea
/tests/coverage
/tests/coverage
composer.lock
8 changes: 4 additions & 4 deletions src/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Client
protected $cluster;

/**
* Tells client to send queries over whole cluster selecting server by random
* Tells client to send queries over whole cluster selecting server by random.
*
* @var bool
*/
Expand Down Expand Up @@ -85,7 +85,7 @@ public function __construct($server, QueryMapperInterface $mapper = null, Transp
}

/**
* Sets flag to use random server in cluster
* Sets flag to use random server in cluster.
*
* @param bool $flag
*/
Expand All @@ -95,7 +95,7 @@ public function useRandomServer(bool $flag)
}

/**
* Returns flag which tells client to use random server in cluster
* Returns flag which tells client to use random server in cluster.
*
* @return bool
*/
Expand Down Expand Up @@ -197,7 +197,7 @@ public function getServer(): Server
}

/**
* Returns random server from cluster
* Returns random server from cluster.
*
* @return \Tinderbox\Clickhouse\Server
*/
Expand Down
4 changes: 2 additions & 2 deletions src/Exceptions/QueryMapperException.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

class QueryMapperException extends \Exception
{
public static function wrongBindingsNumber($inQuery, $count)
public static function wrongBindingsNumber(int $inQuery, int $count): self
{
return new static('Wrong bindings count. Bindings found in query: '.$inQuery.' and given '.$count);
}

public static function multipleBindingsType()
public static function multipleBindingsType(): self
{
return new static('Both named and unnamed bindings found');
}
Expand Down
7 changes: 4 additions & 3 deletions src/Query/Mapper/UnnamedMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,18 @@ class UnnamedMapper extends AbstractMapper implements QueryMapperInterface
* @param string $query
* @param array $bindings
*
* @throws QueryMapperException
*
* @return string
*/
public function bind(string $query, array $bindings): string
{
$this->checkBindings($query, $bindings);
$escapedBindings = $this->escapeBindings($bindings);

$query = str_replace('%', '%%', $query);
$query = str_replace('?', '%s', $query);
$query = str_replace(['%', '?'], ['%%', '%s'], $query);

$query = call_user_func_array('sprintf', array_merge([$query], $escapedBindings));
$query = sprintf($query, ...$escapedBindings);

return $query;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Query/Result.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public function current()

public function next()
{
++$this->current;
$this->current++;
}

public function key()
Expand Down
36 changes: 19 additions & 17 deletions src/Transport/ClickhouseCLIClientTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
class ClickhouseCLIClientTransport implements TransportInterface
{
/**
* Path to executable clickhouse cli client
* Path to executable clickhouse cli client.
*
* @var string
*/
protected $executablePath;

/**
* Last execute query
* Last execute query.
*
* @var string
*/
Expand Down Expand Up @@ -80,8 +80,9 @@ public function send(Server $server, string $query): bool
* @param array $files
* @param int $concurrency
*
* @return array
* @throws \Tinderbox\Clickhouse\Exceptions\ClientException
*
* @return array
*/
public function sendAsyncFilesWithQuery(Server $server, string $query, array $files, int $concurrency = 5): array
{
Expand Down Expand Up @@ -159,7 +160,7 @@ public function getAsync(Server $server, array $queries, int $concurrency = 5):
}

/**
* Puts query in tmp file
* Puts query in tmp file.
*
* @param string $query
*
Expand All @@ -178,7 +179,7 @@ protected function writeQueryInFile(string $query) : string
}

/**
* Removes tmp file with query
* Removes tmp file with query.
*
* @param string $fileName
*/
Expand All @@ -188,7 +189,7 @@ protected function removeQueryFile(string $fileName)
}

/**
* Builds command for write
* Builds command for write.
*
* @param \Tinderbox\Clickhouse\Server $server
* @param string $query
Expand All @@ -203,22 +204,22 @@ protected function buildCommandForWrite(Server $server, string $query, string $f
$command = [];

if (!is_null($file)) {
$command[] = "cat ".$file.' |';
$command[] = 'cat '.$file.' |';
}

$command = array_merge($command, [
$this->executablePath,
"--host='{$server->getHost()}'",
"--port='{$server->getPort()}'",
"--database='{$server->getDatabase()}'",
"--query={$query}"
"--query={$query}",
]);

return implode(' ', $command);
}

/**
* Builds command to read
* Builds command to read.
*
* @param \Tinderbox\Clickhouse\Server $server
* @param string $query
Expand All @@ -236,7 +237,7 @@ protected function buildCommandForRead(Server $server, string $query, $tables =
"--host='{$server->getHost()}'",
"--port='{$server->getPort()}'",
"--database='{$server->getDatabase()}'",
"--max_query_size=".strlen($query),
'--max_query_size='.strlen($query),
];

if ($tables instanceof TempTable || !empty($tables)) {
Expand Down Expand Up @@ -264,21 +265,22 @@ protected function parseTempTable(TempTable $table)
list($structure, $withColumns) = $this->assembleTempTableStructure($table);

return [
"--external",
'--external',
($withColumns ? '--structure=' : '--types=')."'{$structure}'",
"--format='{$table->getFormat()}'",
"--name='{$table->getName()}'",
"--file='{$table->getSource()}'"
"--file='{$table->getSource()}'",
];
}

/**
* Executes command and catches result or error via stdout and stderr
* Executes command and catches result or error via stdout and stderr.
*
* @param string $command
*
* @return string
* @throws \Tinderbox\Clickhouse\Exceptions\ClientException
*
* @return string
*/
protected function executeCommand(string $command) : string
{
Expand Down Expand Up @@ -307,7 +309,7 @@ protected function executeCommand(string $command) : string
}

/**
* Sets last executed query
* Sets last executed query.
*
* @param string $query
*/
Expand All @@ -317,7 +319,7 @@ protected function setLastQuery(string $query)
}

/**
* Returns last executed query
* Returns last executed query.
*
* @return string
*/
Expand All @@ -327,7 +329,7 @@ protected function getLastQuery() : string
}

/**
* Cleans string from tabs and spaces
* Cleans string from tabs and spaces.
*
* @param string $string
*
Expand Down
9 changes: 4 additions & 5 deletions src/Transport/HttpTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,9 @@ public function sendAsyncFilesWithQuery(Server $server, string $query, array $fi
$requests = function ($files) use ($server, $query) {
foreach ($files as $file) {
$headers = array_merge($this->getHeaders(), [
'Content-Length' => null
'Content-Length' => null,
]);


$request = new Request(
'POST', $this->buildRequestUri($server, ['query' => $query]), $headers, $this->getFileHandle($file)
);
Expand All @@ -160,7 +159,7 @@ public function sendAsyncFilesWithQuery(Server $server, string $query, array $fi
'rejected' => $this->parseReason(),
'options' => [
'connect_timeout' => $server->getOptions()->getTimeout(),
'expect' => false
'expect' => false,
],
]
);
Expand Down Expand Up @@ -361,7 +360,7 @@ public function getAsync(Server $server, array $queries, int $concurrency = 5):
'rejected' => $this->parseReason(),
'options' => [
'connect_timeout' => $server->getOptions()->getTimeout(),
'expect' => false
'expect' => false,
],
]
);
Expand Down Expand Up @@ -414,7 +413,7 @@ protected function assembleResult(ResponseInterface $response): Result
protected function buildRequestUri(Server $server, array $query = []): string
{
$uri = $server->getOptions()->getProtocol().'://'.$server->getHost().':'.$server->getPort();

if (!is_null($server->getDatabase())) {
$query['database'] = $server->getDatabase();
}
Expand Down
12 changes: 6 additions & 6 deletions tests/ClientExceptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,42 +10,42 @@
*/
class ClientExceptionsTest extends TestCase
{
public function testInvalidServerProvided()
public function testInvalidServerProvided(): void
{
$e = ClientException::invalidServerProvided('null');

$this->assertInstanceOf(ClientException::class, $e);
}

public function testClusterIsNotProvided()
public function testClusterIsNotProvided(): void
{
$e = ClientException::clusterIsNotProvided();

$this->assertInstanceOf(ClientException::class, $e);
}

public function testConnectionError()
public function testConnectionError(): void
{
$e = ClientException::connectionError();

$this->assertInstanceOf(ClientException::class, $e);
}

public function testServerReturnedError()
public function testServerReturnedError(): void
{
$e = ClientException::serverReturnedError('Syntax error');

$this->assertInstanceOf(ClientException::class, $e);
}

public function testMalformedResponseFromServer()
public function testMalformedResponseFromServer(): void
{
$e = ClientException::malformedResponseFromServer('some text');

$this->assertInstanceOf(ClientException::class, $e);
}

public function testInsertFileNotFound()
public function testInsertFileNotFound(): void
{
$e = ClientException::insertFileNotFound('/tmp/test.csv');

Expand Down
Loading