Commit a211cc15 authored by Mike Ryan's avatar Mike Ryan

#77: Fix most coverage holes in tests (main thing missing is rollback-related stuff).

parent 64aa646e
Pipeline #59194694 passed with stage
in 2 minutes and 20 seconds
......@@ -78,13 +78,13 @@ The MIT License (MIT). Please see [License File](LICENSE.md) for more informatio
[ico-version]: https://img.shields.io/packagist/v/soong/soong.svg?style=flat-square
[ico-license]: https://img.shields.io/badge/license-MIT-brightgreen.svg?style=flat-square
[ico-travis]: https://img.shields.io/travis/soong/soong/master.svg?style=flat-square
[ico-scrutinizer]: https://img.shields.io/scrutinizer/coverage/g/soong/soong.svg?style=flat-square
[ico-code-quality]: https://img.shields.io/scrutinizer/g/soong/soong.svg?style=flat-square
[ico-scrutinizer]: https://img.shields.io/scrutinizer/coverage/gl/soong/soongetl/soong.svg?style=flat-square
[ico-code-quality]: https://img.shields.io/scrutinizer/quality/gl/soong/soongetl/soong.svg?style=flat-square
[ico-downloads]: https://img.shields.io/packagist/dt/soong/soong.svg?style=flat-square
[link-packagist]: https://packagist.org/packages/soong/soong
[link-scrutinizer]: https://scrutinizer-ci.com/g/soong/soong/code-structure
[link-code-quality]: https://scrutinizer-ci.com/g/soong/soong
[link-scrutinizer]: https://scrutinizer-ci.com/gl/soong/soongetl/soong/code-structure
[link-code-quality]: https://scrutinizer-ci.com/gl/soong/soongetl/soong/
[link-downloads]: https://packagist.org/packages/soong/soong
[link-author]: https://gitlab.com/mikeryan
[link-contributors]: ../../contributors
......@@ -21,6 +21,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `PropertyFactory` and `RecordFactory` interfaces/classes added for creation of `Property` and `Record` instances.
- Added basic console command tests.
- `property_factory` configuration option added to `EtlTask`, `LoaderBase`.
- `ExtractorException`, `KeyMapException`, and `LoaderException` classes added.
- Unit test for `Record` added.
## [0.5.3] - 2019-04-12
......
<?php
declare(strict_types=1);
namespace Soong\Contracts\Exception;
/**
* Base for all exceptions thrown by/about extractors.
*/
class ExtractorException extends \RuntimeException implements SoongException
{
}
<?php
declare(strict_types=1);
namespace Soong\Contracts\Exception;
/**
* Base for all exceptions thrown by/about key maps.
*/
class KeyMapException extends \RuntimeException implements SoongException
{
}
<?php
declare(strict_types=1);
namespace Soong\Contracts\Exception;
/**
* Base for all exceptions thrown by/about loaders.
*/
class LoaderException extends \RuntimeException implements SoongException
{
}
......@@ -29,6 +29,8 @@ interface Extractor extends ConfigurableComponent
*
* @return Record[]
* One data record from the source being extracted.
*
* @throws \Soong\Contracts\Exception\ExtractorException
*/
public function extractAll() : iterable;
......
......@@ -16,9 +16,10 @@ interface KeyMap extends ConfigurableComponent, \Countable
*
* @param array $extractedKey
* Extracted key values, keyed by key names.
*
* @param array $loadedKey
* Loaded key values, keyed by key names.
*
* @throws \Soong\Contracts\Exception\KeyMapException
*/
public function saveKeyMap(array $extractedKey, array $loadedKey) : void;
......
......@@ -17,6 +17,8 @@ interface Loader extends ConfigurableComponent
*
* @param Record $data
* Data to be loaded into the destination.
*
* @throws \Soong\Contracts\Exception\LoaderException
*/
public function load(Record $data) : void;
......@@ -49,6 +51,8 @@ interface Loader extends ConfigurableComponent
*
* @param array $key
* Unique key of the destination record to be removed.
*
* @throws \Soong\Contracts\Exception\LoaderException
*/
public function delete(array $key) : void;
}
......@@ -4,7 +4,6 @@ declare(strict_types=1);
namespace Soong\DBAL;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\DriverManager;
/**
......@@ -25,17 +24,14 @@ trait DBAL
/**
* Make (if necessary) and return the connection to the specified database.
*
* @return Connection|null
* @return Connection
*
* @throws \Doctrine\DBAL\DBALException
*/
protected function connection() : ?Connection
protected function connection() : Connection
{
if (empty($this->connection)) {
try {
$this->connection = DriverManager::getConnection($this->getConfigurationValue('connection'));
} catch (DBALException $e) {
print $e->getMessage();
return null;
}
$this->connection = DriverManager::getConnection($this->getConfigurationValue('connection'));
}
return $this->connection;
}
......
......@@ -5,8 +5,7 @@ namespace Soong\Extractor;
use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\FetchMode;
use Soong\Contracts\Data\Record;
use Soong\Contracts\Data\RecordFactory;
use Soong\Contracts\Exception\ExtractorException;
/**
* Extractor for DBAL SQL queries.
......@@ -38,15 +37,21 @@ class DBAL extends CountableExtractorBase
*/
public function extractAll() : iterable
{
try {
$connection = $this->connection();
} catch (DBALException $e) {
throw new ExtractorException('Unable to connect to database.', 0, $e);
}
try {
// @todo: don't accept raw SQL from configuration
/** @var \Doctrine\DBAL\Driver\Statement $statement */
$statement = $this->connection()->executeQuery($this->getConfigurationValue('query'));
while ($row = $statement->fetch(FetchMode::ASSOCIATIVE)) {
yield $this->getConfigurationValue('record_factory')->create($row);
}
$query = $this->getConfigurationValue('query');
$statement = $connection->executeQuery($query);
} catch (DBALException $e) {
// @todo
throw new ExtractorException("Error executing query '$query'.", 0, $e);
}
while ($row = $statement->fetch(FetchMode::ASSOCIATIVE)) {
yield $this->getConfigurationValue('record_factory')->create($row);
}
$this->connection->close();
unset($this->connection);
......
......@@ -6,6 +6,7 @@ namespace Soong\KeyMap;
use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\FetchMode;
use Doctrine\DBAL\Schema\Table;
use Soong\Contracts\Exception\KeyMapException;
/**
* Implementation of key maps using DBAL for storage.
......@@ -164,11 +165,18 @@ class DBAL extends KeyMapBase
* @internal
*
* Create the key map table if it doesn't already exist.
*
* @throws \Soong\Contracts\Exception\KeyMapException
*/
protected function createTable() : void
{
if (!$this->tableExists()) {
$schema = $this->connection()->getSchemaManager();
try {
$schema = $this->connection()->getSchemaManager();
}
catch (DBALException $e) {
throw new KeyMapException('Unable to connect to database.', 0, $e);
}
try {
$map = new Table($this->getConfigurationValue('table'));
$sampleHash = $this->hashKeys(['foo', 'bar']);
......@@ -199,7 +207,7 @@ class DBAL extends KeyMapBase
}
$schema->createTable($map);
} catch (DBALException $e) {
print $e->getMessage();
throw new KeyMapException('Unable to create key map table.', 0, $e);
}
}
}
......@@ -214,7 +222,12 @@ class DBAL extends KeyMapBase
*/
protected function tableExists() : bool
{
$schema = $this->connection()->getSchemaManager();
try {
$schema = $this->connection()->getSchemaManager();
}
catch (DBALException $e) {
throw new KeyMapException('Unable to connect to database.', 0, $e);
}
return $schema->tablesExist([$this->getConfigurationValue('table')]);
}
......
......@@ -5,8 +5,7 @@ namespace Soong\Loader;
use Doctrine\DBAL\DBALException;
use Soong\Contracts\Data\Record;
use Soong\Data\Property;
use Soong\Data\PropertyFactory;
use Soong\Contracts\Exception\LoaderException;
/**
* Loader for DBAL SQL tables.
......@@ -39,11 +38,17 @@ class DBAL extends LoaderBase
public function load(Record $data) : void
{
try {
$this->connection()->insert(
$this->getConfigurationValue('table'),
$connection = $this->connection();
} catch (DBALException $e) {
throw new LoaderException('Unable to connect to database.', 0, $e);
}
try {
$table = $this->getConfigurationValue('table');
$connection->insert(
$table,
$data->toArray()
);
$id = $this->connection()->lastInsertId();
$id = $connection->lastInsertId();
if ($id) {
$keyKeys = array_keys($this->getKeyProperties());
$data->setProperty(
......@@ -52,7 +57,7 @@ class DBAL extends LoaderBase
);
}
} catch (DBALException $e) {
print $e->getMessage();
throw new LoaderException("Unable to insert into $table.", 0, $e);
}
}
......@@ -61,7 +66,12 @@ class DBAL extends LoaderBase
*/
public function delete(array $key) : void
{
$queryBuilder = $this->connection()->createQueryBuilder();
try {
$connection = $this->connection();
} catch (DBALException $e) {
throw new LoaderException('Unable to connect to database.', 0, $e);
}
$queryBuilder = $connection->createQueryBuilder();
$queryBuilder->delete($this->getConfigurationValue('table'));
$counter = 0;
foreach ($key as $name => $value) {
......@@ -70,7 +80,7 @@ class DBAL extends LoaderBase
$counter++;
}
$queryBuilder->execute();
$this->connection->close();
$connection->close();
$this->connection = null;
}
}
......@@ -18,7 +18,7 @@ use Soong\Contracts\Data\PropertyFactory;
* }
* @endcode
*/
abstract class DataPropertyTestBase extends TestCase
abstract class PropertyTestBase extends TestCase
{
/**
......
<?php
namespace Soong\Tests\Contracts\Data;
use PHPUnit\Framework\TestCase;
use Soong\Contracts\Data\RecordFactory;
use Soong\Data\PropertyFactory;
/**
* Base class for testing Record implementations.
*
* To test a record class, simply extend this class and implement
* setUp(), assigning the fully-qualified class name to recordFactoryClass:
*
* @code
* protected function setUp() : void
* {
* $this->recordFactoryClass = '\Soong\Data\RecordFactory';
* }
* @endcode
*/
abstract class RecordTestBase extends TestCase
{
/**
* Fully-qualified name of a RecordFactory implementation.
*
* @var RecordFactory $recordFactoryClass
*/
protected $recordFactoryClass;
/**
* Provides record data to test.
*
* @return array
*/
public function recordDataProvider() : array
{
$basic_data = [
'string_property' => 'value1',
'int_property' => 0,
'null_property' => null,
];
$data['string property'] = [
$basic_data, // Data for initializing a record.
'string_property', // Property to get.
'value1', // Expected value of the property.
true, // Expected existence of the property.
];
$data['int property'] = [
$basic_data,
'int_property',
0,
true,
];
$data['null property'] = [
$basic_data,
'null_property',
null,
true,
];
$data['missing property'] = [
$basic_data,
'i_do_not_exist',
null,
false,
];
return $data;
}
/**
* Test toArray(), getProperty() and propertyExists().
*
* @dataProvider recordDataProvider
*
* @param array $data
* The data array for initially constructing the record.
* @param string $propertyName
* Name of property to test.
* @param mixed $expectedPropertyValue
* Expected result from getProperty($propertyName).
* @param bool $expectedPropertyExists
* Expected result from propertyExists($propertyName).
*/
public function testRecord(array $data, string $propertyName, $expectedPropertyValue, bool $expectedPropertyExists)
{
// @todo Mock the property factory.
$propertyFactory = new PropertyFactory();
/** @var RecordFactory $factory */
$factory = new $this->recordFactoryClass($propertyFactory);
$record = $factory->create($data);
$this->assertEquals($data, $record->toArray(), 'Record retrieved');
$this->assertEquals($expectedPropertyValue,
$record->getProperty($propertyName)->getValue(), 'Property retrieved');
$this->assertEquals($expectedPropertyExists,
$record->propertyExists($propertyName), "Property existence identified");
}
/**
* Provides record data for testing setProperty().
*
* @return array
*/
public function setPropertyDataProvider() : array
{
$basic_data = [
'string_property' => 'value1',
'int_property' => 0,
'null_property' => null,
];
$data['overwrite existing property'] = [
$basic_data, // Data for initializing a record.
'string_property', // Property to set.
'value2', // Value to set it to.
];
$data['new property'] = [
$basic_data,
'new_property',
5,
];
$data['null property'] = [
[],
'null_property',
null,
];
return $data;
}
/**
* Test setProperty.
*
* @dataProvider setPropertyDataProvider
*
* @param array $data
* The data array for initially constructing the record.
* @param string $propertyName
* Name of property to set.
* @param mixed $propertyValue
* Value to set the property (and to expect to be retrieved).
* @param bool $expectedPropertyExists
* Expected result from propertyExists($propertyName).
*/
public function testSetProperty(array $data, string $propertyName, $propertyValue)
{
// @todo Mock the property factory.
$propertyFactory = new PropertyFactory();
/** @var RecordFactory $factory */
$factory = new $this->recordFactoryClass($propertyFactory);
$record = $factory->create($data);
$record->setProperty($propertyName, $propertyFactory->create($propertyValue));
$this->assertEquals($propertyValue,
$record->getProperty($propertyName)->getValue(), 'Property retrieved');
$this->assertEquals(true,
$record->propertyExists($propertyName), "Property existence identified");
}
}
......@@ -162,6 +162,27 @@ abstract class KeyMapTestBase extends TestCase
[],
],
],
'empty map' => [
'key_metadata' => [
'extractor_keys' => [
'sourceid' => ['type' => 'integer'],
],
'loader_keys' => [
'destid' => ['type' => 'integer'],
],
],
'map_data' => [],
'extracted_keys_to_lookup' => [
[1],
[3],
[4],
],
'expected_loaded_keys' => [
[],
[],
[],
],
],
];
}
......@@ -188,11 +209,14 @@ abstract class KeyMapTestBase extends TestCase
// Populate the key map.
$configuration = array_merge($this->configuration, $keyMetadata);
/** @var KeyMap $keyMap */
$keyMap = new $this->keyMapClass($configuration);
foreach ($mapData as $keyList) {
$keyMap->saveKeyMap($keyList['extracted_key'], $keyList['loaded_key']);
}
$this->assertEquals(count($mapData), $keyMap->count());
// Check each key pair.
foreach ($loadedKeysToLookup as $index => $loadedKey) {
$extractedKey = $keyMap->lookupExtractedKeys($loadedKey);
......@@ -299,6 +323,27 @@ abstract class KeyMapTestBase extends TestCase
[],
],
],
'empty map' => [
'key_metadata' => [
'extractor_keys' => [
'sourceid' => ['type' => 'integer'],
],
'loader_keys' => [
'destid' => ['type' => 'integer'],
],
],
'map_data' => [],
'loaded_keys_to_lookup' => [
[5],
[6],
[2],
],
'expected_extracted_keys' => [
[],
[],
[],
],
],
];
}
......@@ -326,7 +371,9 @@ abstract class KeyMapTestBase extends TestCase
// Check each key pair.
foreach ($keysToDelete as $extractedKey) {
$loadedKey = $keyMap->lookupLoadedKey($extractedKey);
$this->assertNotEmpty($loadedKey);
if (!empty($mapData)) {
$this->assertNotEmpty($loadedKey);
}
$keyMap->delete($extractedKey);
$loadedKey = $keyMap->lookupLoadedKey($extractedKey);
$this->assertEmpty($loadedKey);
......@@ -403,13 +450,50 @@ abstract class KeyMapTestBase extends TestCase
[4],
],
],
'empty table' => [
'key_metadata' => [
'extractor_keys' => [
'sourceid' => ['type' => 'integer'],
],
'loader_keys' => [
'destid' => ['type' => 'integer'],
],
],
'map_data' => [],
'keys_to_delete' => [
[1],
[4],
],
],
];
}
/*
The KeyMap should itself be an iterator, test when that is implemented.
public function testIterate()
/**
* Test iteration of the KeyMap.
*
* @dataProvider lookupExtractedKeysDataProvider
*/
public function testIterate(
array $keyMetadata,
array $mapData,
array $loadedKeysToLookup,
array $expectedExtractedKeys
) : void
{
// Populate the key map.
$configuration = array_merge($this->configuration, $keyMetadata);
/** @var KeyMap $keyMap */
$keyMap = new $this->keyMapClass($configuration);
foreach ($mapData as $keyList) {
$keyMap->saveKeyMap($keyList['extracted_key'], $keyList['loaded_key']);
}
}*/
$this->assertEquals(count($mapData), $keyMap->count());
// Iterate and make sure we get back the same keys.
foreach ($keyMap->iterate() as $index => $mapRecord) {
$this->assertEquals(array_values($mapData[$index]['extracted_key']),
array_values($mapRecord));
}
}
}
......@@ -54,7 +54,7 @@ trait DBALTesting
protected function tearDown() : void
{
// Remove the random db file for this test.
unlink($this->dbFile);
@unlink($this->dbFile);
parent::tearDown();
}
}
......@@ -2,12 +2,12 @@
namespace Soong\Tests\Data;
use Soong\Tests\Contracts\Data\DataPropertyTestBase;
use Soong\Tests\Contracts\Data\PropertyTestBase;
/**
* Tests the \Soong\Data\Property class.
*/
class PropertyTest extends DataPropertyTestBase