Skip to content

Commit

Permalink
Factor out DatastoreResource class (#4372)
Browse files Browse the repository at this point in the history
  • Loading branch information
dafeder authored Jan 16, 2025
1 parent c6b94a1 commit 63c4ff8
Show file tree
Hide file tree
Showing 23 changed files with 310 additions and 163 deletions.
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(
'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

0 comments on commit 63c4ff8

Please sign in to comment.