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

Factor out DatastoreResource class #4372

Open
wants to merge 23 commits into
base: 2.x
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
30 changes: 23 additions & 7 deletions modules/common/src/DataResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_
* @return \Drupal\common\DataResource
* DataResource object.
*
* @deprecated Use DataResource::createFromEntity() instead.
*
* @see self::createFromEntity()
* @deprecated in dkan:8.x-2.17 and is removed from dkan:8.x-2.21. Use
* DataResource::createFromEntity() instead.
* @see https://github.com/GetDKAN/dkan/pull/4027
*/
public static function createFromRecord(object $record): DataResource {
$resource = new static($record->filePath, $record->mimeType, $record->perspective);
Expand Down Expand Up @@ -236,6 +236,11 @@ public function changeMimeType($newMimeType) {
*
* @return \Drupal\datastore\DatastoreResource
* Datastore Resource.
*
* @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use storage
* classes like DatabaseTable::getTableName() to determine correct table
* names, and pass true to ::getFilePath to get the resolved URL.
* @see https://github.com/GetDKAN/dkan/pull/4372
*/
public function getDatastoreResource(): DatastoreResource {
return new DatastoreResource(
Expand All @@ -253,10 +258,16 @@ public function getIdentifier() {
}

/**
* Getter.
* Get the resource's filepath.
*
* @param bool|null $resolve
* Whether to resolve the URL host tokens in the file path.
*
* @return string
* The file path.
*/
public function getFilePath() {
return $this->filePath;
public function getFilePath(?bool $resolve = FALSE):string {
return $resolve ? UrlHostTokenResolver::resolve($this->getFilePath()) : $this->filePath;
}

/**
Expand Down Expand Up @@ -324,6 +335,11 @@ public function getUniqueIdentifierNoPerspective(): string {

/**
* Retrieve datastore table name for resource.
*
* @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use storage
* classes like DatabaseTable::getTableName() to determine correct table
* names.
* @see https://github.com/GetDKAN/dkan/pull/4372
*/
public function getTableName() {
return 'datastore_' . md5($this->getUniqueIdentifier());
Expand All @@ -333,7 +349,7 @@ public function getTableName() {
* {@inheritDoc}
*/
#[\ReturnTypeWillChange]
public function jsonSerialize() {
public function jsonSerialize(): mixed {
return $this->serialize();
}

Expand Down
14 changes: 3 additions & 11 deletions modules/common/src/Storage/AbstractDatabaseTable.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,13 @@ abstract class AbstractDatabaseTable implements DatabaseTableInterface {
*/
protected $connection;

/**
* Get the full name of datastore db table.
*
* @return string
* Table name.
*/
abstract protected function getTableName();

/**
* Prepare data.
*
* Transform the string data given into what should be use by the insert
* query.
*/
abstract protected function prepareData(string $data, string $id = NULL): array;
abstract protected function prepareData(string $data, ?string $id = NULL): array;

/**
* Get the primary key used in the table.
Expand Down Expand Up @@ -113,7 +105,7 @@ public function retrieveAll(): array {
/**
* Store data.
*/
public function store($data, string $id = NULL): string {
public function store($data, ?string $id = NULL): string {
$this->setTable();

$existing = (isset($id)) ? $this->retrieve($id) : NULL;
Expand Down Expand Up @@ -263,7 +255,7 @@ protected function sanitizedErrorMessage(string $unsanitizedMessage) {
* @throws \Exception
* Throws an exception if the schema was not already set.
*/
protected function setTable() {
public function setTable() {
if (!$this->tableExist($table_name = $this->getTableName())) {
if ($schema = $this->schema) {
try {
Expand Down
1 change: 1 addition & 0 deletions modules/datastore/datastore.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ services:
- '@dkan.datastore.data_dictionary.alter_table_query_builder.mysql'
- '@dkan.metastore.service'
- '@dkan.metastore.data_dictionary_discovery'
- '@dkan.datastore.database_table_factory'
tags:
- { name: resource_processor, priority: 25 }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@ protected function runIt() {
}

// Attempt to resolve resource file name from file path.
if (($file_path = \Drupal::service('file_system')->realpath($this->resource->getFilePath())) === FALSE) {
return $this->setResultError(sprintf('Unable to resolve file name "%s" for resource with identifier "%s".', $this->resource->getFilePath(), $this->resource->getId()));
if (($file_path = \Drupal::service('file_system')->realpath($this->resource->getFilePath(TRUE))) === FALSE) {
return $this->setResultError(sprintf(
dafeder marked this conversation as resolved.
Show resolved Hide resolved
'Unable to resolve file name "%s" for resource with identifier "%s".',
$this->resource->getFilePath(TRUE),
$this->resource->getUniqueIdentifier())
);
}

$size = @filesize($file_path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Drupal\Tests\datastore_mysql_import\Kernel\Storage;

use Drupal\datastore\DatastoreResource;
use Drupal\common\DataResource;
use Drupal\datastore_mysql_import\Storage\MySqlDatabaseTable;
use Drupal\KernelTests\KernelTestBase;

Expand All @@ -11,6 +11,7 @@
* @coversDefaultClass \Drupal\datastore_mysql_import\Storage\MySqlDatabaseTableFactory
*
* @group datastore_mysql_import
* @group kernel
*/
class MySqlDatabaseTableFactoryTest extends KernelTestBase {

Expand All @@ -33,10 +34,8 @@ public function testFactoryServiceResourceException() {
}

public function testFactoryService() {
$identifier = 'identifier';
$file_path = dirname(__FILE__, 4) . '/data/columnspaces.csv';
$datastore_resource = new DatastoreResource(
$identifier,
$datastore_resource = new DataResource(
$file_path,
'text/csv'
);
Expand Down
27 changes: 22 additions & 5 deletions modules/datastore/src/DatastoreResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
/**
* Basic datastore resource class.
*
* Always generate this object using DataResource::getDatastoreResource().
*
* @see \Drupal\common\DataResource::getDatastoreResource()
* @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
* \Drupal\common\DataResource instead.
* @see https://github.com/GetDKAN/dkan/pull/4372
*/
class DatastoreResource implements \JsonSerializable {

Expand Down Expand Up @@ -43,20 +43,38 @@ public function __construct($id, $file_path, $mime_type) {

/**
* Get the resource ID.
*
* @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
* \Drupal\common\DataResource::getUniqueIdentifier() instead.
* @see https://github.com/GetDKAN/dkan/pull/4372
*/
public function getId(): string {
return $this->id;
}

/**
* Get the file path.
*
* @return string
* The file path.
*
* @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
* \Drupal\common\DataResource::getUniqueIdentifier() instead.
* @see https://github.com/GetDKAN/dkan/pull/4372
*/
public function getFilePath(): string {
return $this->filePath;
}

/**
* Get the mimeType.
*
* @return string
* The mimeType.
*
* @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
* \Drupal\common\DataResource::getMimeType() instead.
* @see https://github.com/GetDKAN/dkan/pull/4372
*/
public function getMimeType(): string {
return $this->mimeType;
Expand All @@ -65,8 +83,7 @@ public function getMimeType(): string {
/**
* {@inheritdoc}
*/
#[\ReturnTypeWillChange]
public function jsonSerialize() {
public function jsonSerialize(): mixed {
return (object) [
'filePath' => $this->getFilePath(),
'id' => $this->getId(),
Expand Down
6 changes: 3 additions & 3 deletions modules/datastore/src/Plugin/QueueWorker/ImportJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class ImportJob extends AbstractPersistentJob {
/**
* Datastore resource.
*
* @var \Drupal\datastore\DatastoreResource
* @var \Drupal\common\DataResource
*/
protected $resource;

Expand All @@ -105,7 +105,7 @@ class ImportJob extends AbstractPersistentJob {
* @param array|null $config
* Configuration options.
*/
protected function __construct(string $identifier, $storage, array $config = NULL) {
protected function __construct(string $identifier, $storage, ?array $config = NULL) {
parent::__construct($identifier, $storage, $config);

$this->dataStorage = $config['storage'];
Expand Down Expand Up @@ -195,7 +195,7 @@ public function getStorage() {
* {@inheritdoc}
*/
protected function runIt() {
$filename = $this->resource->getFilePath();
$filename = $this->resource->getFilePath(TRUE);
$size = @filesize($filename);
if (!$size) {
return $this->setResultError("Can't get size from file {$filename}");
Expand Down
17 changes: 8 additions & 9 deletions modules/datastore/src/Service/ImportService.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,9 @@ public function import() {
$data_resource = $this->getResource();

if ($result->getStatus() === Result::ERROR) {
$datastore_resource = $data_resource->getDatastoreResource();
$this->logger->error('Error importing resource id:%id path:%path message:%message', [
'%id' => $datastore_resource->getId(),
'%path' => $datastore_resource->getFilePath(),
'%id' => $data_resource->getUniqueIdentifier(),
'%path' => $data_resource->getFilePath(TRUE),
'%message' => $result->getError(),
]);
}
Expand Down Expand Up @@ -188,20 +187,20 @@ public function getImporter(): ImportJob {
if ($this->importJob ?? FALSE) {
return $this->importJob;
}
$datastore_resource = $this->getResource()->getDatastoreResource();
$data_resource = $this->getResource();

$delimiter = ",";
if ($datastore_resource->getMimeType() == 'text/tab-separated-values') {
if ($data_resource->getMimeType() == 'text/tab-separated-values') {
$delimiter = "\t";
}

$this->importJob = call_user_func([$this->importerClass, 'get'],
$datastore_resource->getId(),
$data_resource->getUniqueIdentifier(),
$this->importJobStoreFactory->getInstance(),
[
"storage" => $this->getStorage(),
"parser" => $this->getNonRecordingParser($delimiter),
"resource" => $datastore_resource,
"resource" => $data_resource,
]
);

Expand Down Expand Up @@ -245,8 +244,8 @@ private function getNonRecordingParser(string $delimiter) : Csv {
* DatabaseTable storage object.
*/
public function getStorage(): DatabaseTable {
$datastore_resource = $this->getResource()->getDatastoreResource();
return $this->databaseTableFactory->getInstance($datastore_resource->getId(), ['resource' => $datastore_resource]);
$data_resource = $this->getResource();
return $this->databaseTableFactory->getInstance($data_resource->getUniqueIdentifier(), ['resource' => $data_resource]);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Drupal\common\DataResource;
use Drupal\datastore\DataDictionary\AlterTableQueryBuilderInterface;
use Drupal\datastore\Service\ResourceProcessorInterface;
use Drupal\datastore\Storage\DatabaseTableFactory;
use Drupal\metastore\MetastoreService;
use Drupal\metastore\DataDictionary\DataDictionaryDiscoveryInterface;

Expand Down Expand Up @@ -43,6 +44,13 @@ class DictionaryEnforcer implements ResourceProcessorInterface {
*/
protected $resourceMapper;

/**
* Database table factory service.
*
* @var \Drupal\datastore\Storage\DatabaseTableFactory
*/
protected $databaseTableFactory;

/**
* Constructs a \Drupal\Component\Plugin\PluginBase object.
*
Expand All @@ -52,15 +60,19 @@ class DictionaryEnforcer implements ResourceProcessorInterface {
* The metastore service.
* @param \Drupal\metastore\DataDictionary\DataDictionaryDiscoveryInterface $data_dictionary_discovery
* The data-dictionary discovery service.
* @param \Drupal\datastore\Storage\DatabaseTableFactory $table_factory
* The datastore database table factory service.
*/
public function __construct(
AlterTableQueryBuilderInterface $alter_table_query_builder,
MetastoreService $metastore,
DataDictionaryDiscoveryInterface $data_dictionary_discovery
DataDictionaryDiscoveryInterface $data_dictionary_discovery,
DatabaseTableFactory $table_factory,
) {
$this->metastore = $metastore;
$this->dataDictionaryDiscovery = $data_dictionary_discovery;
$this->alterTableQueryBuilder = $alter_table_query_builder;
$this->databaseTableFactory = $table_factory;
}

/**
Expand All @@ -78,7 +90,8 @@ public function process(DataResource $resource): void {
// Get data-dictionary for the given resource.
$dictionary = $this->getDataDictionaryForResource($resource);
// Retrieve name of datastore table for resource.
$datastore_table = $resource->getTableName();
$table = $this->databaseTableFactory->getInstance('', ['resource' => $resource]);
$datastore_table = $table->getTableName();

$this->applyDictionary($dictionary, $datastore_table);
}
Expand Down Expand Up @@ -125,13 +138,13 @@ public function applyDictionary(RootedJsonData $dictionary, string $datastore_ta
/**
* Returning data dictionary fields from schema.
*
* @param string $identifier
* @param string|null $identifier
* A resource's identifier. Used when in reference mode.
*
* @return array|null
* An array of dictionary fields or null if no dictionary is in use.
*/
public function returnDataDictionaryFields(string $identifier = NULL): ?array {
public function returnDataDictionaryFields(?string $identifier = NULL): ?array {
// Get data dictionary mode.
$dd_mode = $this->dataDictionaryDiscovery->getDataDictionaryMode();
// Get data dictionary info.
Expand Down
Loading