diff --git a/modules/common/src/DataResource.php b/modules/common/src/DataResource.php index ff31e3f63c..07e9a113b4 100644 --- a/modules/common/src/DataResource.php +++ b/modules/common/src/DataResource.php @@ -3,7 +3,6 @@ namespace Drupal\common; use Drupal\datastore\DatastoreResource; -use Procrastinator\HydratableTrait; use Procrastinator\JsonSerializeTrait; /** @@ -28,7 +27,7 @@ * @todo Refactor as service. */ class DataResource implements \JsonSerializable { - use HydratableTrait, JsonSerializeTrait; + use JsonSerializeTrait; const DEFAULT_SOURCE_PERSPECTIVE = 'source'; @@ -78,6 +77,13 @@ class DataResource implements \JsonSerializable { /** * Constructor. + * + * @param string $file_path + * Path to the file. + * @param string $mimeType + * File mime type. + * @param string $perspective + * Can be one of "local_file", "local_url", or "source". */ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_SOURCE_PERSPECTIVE) { // @todo generate UUID instead. @@ -91,7 +97,26 @@ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_ } /** - * Create a new version. + * Create a DataResource object from a database record. + * + * @param object $record + * Data resource record from the database. Must contain these properties: + * 'filePath', 'mimeType', 'perspective', 'version'. + * + * @return \Drupal\common\DataResource + * DataResource object. + */ + public static function createFromRecord(object $record): DataResource { + $resource = new static($record->filePath, $record->mimeType, $record->perspective); + // MD5 of record's file path can differ from the MD5 generated in the + // constructor, so we have to explicitly set the identifier. + $resource->identifier = $record->identifier; + $resource->version = $record->version; + return $resource; + } + + /** + * Clone the current resource with a new version identifier. * * Versions are, simply, a unique "string" used to represent changes in a * resource. For example, when new data is added to a file/resource a new @@ -101,17 +126,18 @@ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_ * resources, it simply models the behavior to allow other parts of the * system to create new versions of resources when they deem it necessary. */ - public function createNewVersion() { + public function createNewVersion(): DataResource { $newVersion = time(); if ($newVersion == $this->version) { $newVersion++; } - - return $this->createCommon('version', $newVersion); + $clone = clone $this; + $clone->version = $newVersion; + return $clone; } /** - * Create a new perspective. + * Clone the current resource with a new perspective. * * Perspectives are useful to represent clusters of connected resources. * @@ -121,11 +147,11 @@ public function createNewVersion() { * aware of the new resource, the API endpoint, and maintain the relatioship * between the 2 resources. */ - public function createNewPerspective($perspective, $uri) { - $new = $this->createCommon('perspective', $perspective); - $new->changeFilePath($uri); - - return $new; + public function createNewPerspective($perspective, $uri): DataResource { + $clone = clone $this; + $clone->perspective = $perspective; + $clone->changeFilePath($uri); + return $clone; } /** @@ -142,18 +168,6 @@ public function changeMimeType($newMimeType) { $this->mimeType = $newMimeType; } - /** - * Private. - */ - private function createCommon($property, $value) { - $current = $this->{$property}; - $new = $value; - $this->{$property} = $new; - $newResource = clone $this; - $this->{$property} = $current; - return $newResource; - } - /** * Get object storing datastore specific information about this resource. * @@ -205,6 +219,9 @@ public function getPerspective() { /** * Getter. + * + * @return string + * The unique identifier. */ public function getUniqueIdentifier() { return self::buildUniqueIdentifier($this->identifier, $this->version, $this->perspective); diff --git a/modules/common/tests/src/Unit/ResourceTest.php b/modules/common/tests/src/Unit/DataResourceTest.php similarity index 62% rename from modules/common/tests/src/Unit/ResourceTest.php rename to modules/common/tests/src/Unit/DataResourceTest.php index c636027359..3d20998e2a 100644 --- a/modules/common/tests/src/Unit/ResourceTest.php +++ b/modules/common/tests/src/Unit/DataResourceTest.php @@ -12,8 +12,10 @@ /** * Unit tests for Drupal\common\Resource. + * + * @coversDefaultClass \Drupal\common\DataResource */ -class ResourceTest extends TestCase { +class DataResourceTest extends TestCase { /** * Test getTableName(). @@ -82,4 +84,44 @@ public function testGetIdentifierAndVersionException() { $result = DataResource::getIdentifierAndVersion($id); } + /** + * @covers ::createNewPerspective + */ + public function testCreateNewPerspective() { + $data_resource = new DataResource( + '/foo/bar', + 'txt', + DataResource::DEFAULT_SOURCE_PERSPECTIVE + ); + + $clone_data_resource = $data_resource->createNewPerspective('local_url', 'uri://foo/bar'); + + // Not the same object. + $this->assertNotSame($data_resource, $clone_data_resource); + // Clone contains 'local_url' perspective. + $this->assertEquals('local_url', $clone_data_resource->getPerspective()); + $this->assertEquals('uri://foo/bar', $clone_data_resource->getFilePath()); + } + + /** + * @covers ::createNewVersion + */ + public function testCreateNewVersion() { + $data_resource = new DataResource( + '/foo/bar', + 'txt', + DataResource::DEFAULT_SOURCE_PERSPECTIVE + ); + + $clone_data_resource = $data_resource->createNewVersion(); + + // Not the same object. + $this->assertNotSame($data_resource, $clone_data_resource); + // Clone contains new version. + $this->assertNotEquals( + $data_resource->getVersion(), + $clone_data_resource->getVersion() + ); + } + } diff --git a/modules/datastore/src/DatastoreResource.php b/modules/datastore/src/DatastoreResource.php index 98ef237909..fc04b9c20c 100644 --- a/modules/datastore/src/DatastoreResource.php +++ b/modules/datastore/src/DatastoreResource.php @@ -40,21 +40,21 @@ public function __construct($id, $file_path, $mime_type) { /** * Get the resource ID. */ - public function getId() { + public function getId(): string { return $this->id; } /** * Get the file path. */ - public function getFilePath() { + public function getFilePath(): string { return $this->filePath; } /** * Get the mimeType. */ - public function getMimeType() { + public function getMimeType(): string { return $this->mimeType; } @@ -64,35 +64,10 @@ public function getMimeType() { #[\ReturnTypeWillChange] public function jsonSerialize() { return (object) [ - 'filePath' => $this->filePath, - 'id' => $this->id, - 'mimeType' => $this->mimeType, + 'filePath' => $this->getFilePath(), + 'id' => $this->getId(), + 'mimeType' => $this->getMimeType(), ]; } - /** - * {@inheritdoc} - */ - public static function hydrate($json) { - $data = json_decode($json); - $reflector = new \ReflectionClass(self::class); - $object = $reflector->newInstanceWithoutConstructor(); - - $reflector = new \ReflectionClass($object); - - $p = $reflector->getProperty('filePath'); - $p->setAccessible(TRUE); - $p->setValue($object, $data->filePath); - - $p = $reflector->getProperty('id'); - $p->setAccessible(TRUE); - $p->setValue($object, $data->id); - - $p = $reflector->getProperty('mimeType'); - $p->setAccessible(TRUE); - $p->setValue($object, $data->mimeType); - - return $object; - } - } diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php index ebdd153a42..9615ff698e 100755 --- a/modules/datastore/src/Storage/DatabaseTable.php +++ b/modules/datastore/src/Storage/DatabaseTable.php @@ -70,16 +70,6 @@ public function jsonSerialize() { return (object) ['resource' => $this->resource]; } - /** - * Hydrate. - */ - public static function hydrate(string $json) { - $data = json_decode($json); - $resource = DatastoreResource::hydrate(json_encode($data->resource)); - - return new DatabaseTable(\Drupal::service('database'), $resource); - } - /** * Get the full name of datastore db table. * diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php index 5073538644..e5207f3dc4 100644 --- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php +++ b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php @@ -13,6 +13,9 @@ /** * Unit tests for Importer class. + * + * @group datastore + * @group dkan-core */ class ImportJobTest extends TestCase { @@ -34,7 +37,7 @@ protected function tearDown(): void { /** * */ - private function getDatastore(DatastoreResource $resource) { + private function getDatastore(DatastoreResource $resource): ImportJob { $storage = new Memory(); $config = [ "resource" => $resource, @@ -49,7 +52,7 @@ private function getDatastore(DatastoreResource $resource) { */ public function testBasics() { $resource = new DatastoreResource(1, __DIR__ . "/../../../../data/countries.csv", "text/csv"); - $this->assertEquals($resource->getID(), 1); + $this->assertEquals(1, $resource->getID()); $datastore = $this->getDatastore($resource); @@ -145,12 +148,16 @@ public function testColumnNameSpaces() { } /** + * Test JSON/hydrate round-trip. * + * This pattern is deprecated. + * + * @group legacy */ public function testSerialization() { $timeLimit = 40; $resource = new DatastoreResource(1, __DIR__ . "/../../../../data/countries.csv", "text/csv"); - $this->assertEquals($resource->getID(), 1); + $this->assertEquals(1, $resource->getID()); $datastore = $this->getDatastore($resource); $datastore->setTimeLimit($timeLimit); @@ -216,17 +223,14 @@ public function testMultiplePasses() { */ public function testBadStorage() { $storageInterfaceClass = DatabaseTableInterface::class; - $this->expectExceptionMessage("Storage must be an instance of {$storageInterfaceClass}"); + $this->expectExceptionMessage("Storage must be an instance of $storageInterfaceClass"); $resource = new DatastoreResource(1, __DIR__ . "/../../../../data/countries.csZv", "text/csv"); - $importer = ImportJob::get("1", new Memory(), [ + ImportJob::get("1", new Memory(), [ "resource" => $resource, "storage" => new TestMemStorageBad(), "parser" => Csv::getParser(), ]); - - $json = json_encode($importer); - ImportJob::hydrate($json); } /** @@ -235,7 +239,7 @@ public function testBadStorage() { public function testNonStorage() { $this->expectExceptionMessage("Storage must be an instance of Drupal\common\Storage\DatabaseTableInterface"); $resource = new DatastoreResource(1, __DIR__ . "/../../../../data/countries.csv", "text/csv"); - $importer = ImportJob::get("1", new Memory(), [ + ImportJob::get("1", new Memory(), [ "resource" => $resource, "storage" => new class { }, @@ -243,7 +247,7 @@ public function testNonStorage() { ]); } - public function sanitizeDescriptionProvider() { + public function sanitizeDescriptionProvider(): array { return [ 'multiline' => ["Multi\nLine", 'Multi Line'], ]; @@ -272,7 +276,7 @@ public function testSanitizeHeader($column, $expected) { $this->assertEquals($expected, ImportJob::sanitizeHeader($column)); } - public function truncateHeaderProvider() { + public function truncateHeaderProvider(): array { $max_length = 64; return [ 'max_length' => [ diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php index 9744c305ab..a6e65c0d47 100644 --- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php +++ b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php @@ -79,18 +79,6 @@ public function jsonSerialize() return (object) ['storage' => $this->storage]; } - public static function hydrate(string $json) - { - $class = new \ReflectionClass(self::class); - $instance = $class->newInstanceWithoutConstructor(); - - $property = $class->getParentClass()->getProperty('storage'); - $property->setAccessible(true); - $property->setValue($instance, (array) (json_decode($json))->storage); - - return $instance; - } - /** * Clean up and set the schema for SQL storage. * diff --git a/modules/metastore/src/Reference/Referencer.php b/modules/metastore/src/Reference/Referencer.php index 9cbad78d40..b3d41105bf 100644 --- a/modules/metastore/src/Reference/Referencer.php +++ b/modules/metastore/src/Reference/Referencer.php @@ -212,9 +212,19 @@ private function registerWithResourceMapper(string $downloadUrl, string $mimeTyp } /** - * Private. + * Get download URL for existing resource. + * + * @param array $info + * Info. + * @param \Drupal\common\DataResource $stored + * Stored data resource object. + * @param string $mimeType + * MIME type. + * + * @return string + * The download URL. */ - private function handleExistingResource($info, $stored, $mimeType) { + private function handleExistingResource(array $info, DataResource $stored, string $mimeType): string { if ($info[0]->perspective == DataResource::DEFAULT_SOURCE_PERSPECTIVE && (ResourceMapper::newRevision() == 1 || $stored->getMimeType() != $mimeType)) { $new = $stored->createNewVersion(); diff --git a/modules/metastore/src/ResourceMapper.php b/modules/metastore/src/ResourceMapper.php index c40187f4d9..8f0955e509 100644 --- a/modules/metastore/src/ResourceMapper.php +++ b/modules/metastore/src/ResourceMapper.php @@ -13,12 +13,15 @@ * Map resource URLs to local files. */ class ResourceMapper { + use EventDispatcherTrait; const EVENT_REGISTRATION = 'dkan_metastore_resource_mapper_registration'; + const EVENT_RESOURCE_MAPPER_PRE_REMOVE_SOURCE = 'dkan_metastore_pre_remove_source'; const DEREFERENCE_NO = 0; + const DEREFERENCE_YES = 1; /** @@ -58,7 +61,7 @@ public static function newRevision() { * @todo the Resource class currently lives in datastore, we should move it * to a more neutral place. */ - public function register(DataResource $resource) : bool { + public function register(DataResource $resource): bool { $this->filePathExists($resource->getFilePath()); $this->store->store(json_encode($resource)); $this->dispatchEvent(self::EVENT_REGISTRATION, $resource); @@ -133,7 +136,10 @@ protected function validateNewVersion(DataResource $resource) { */ public function get(string $identifier, $perspective = DataResource::DEFAULT_SOURCE_PERSPECTIVE, $version = NULL): ?DataResource { $data = $this->getFull($identifier, $perspective, $version); - return ($data != FALSE) ? DataResource::hydrate(json_encode($data)) : NULL; + if ($data !== FALSE) { + return DataResource::createFromRecord($data); + } + return NULL; } /** @@ -238,7 +244,7 @@ public function filePathExists($filePath) { /** * Private. */ - private function exists($identifier, $perspective, $version = NULL) : bool { + private function exists($identifier, $perspective, $version = NULL): bool { $item = $this->get($identifier, $perspective, $version); return isset($item); } diff --git a/modules/metastore/src/Storage/MetastoreStorageInterface.php b/modules/metastore/src/Storage/MetastoreStorageInterface.php index a7ca14ed5b..2db518b9b2 100644 --- a/modules/metastore/src/Storage/MetastoreStorageInterface.php +++ b/modules/metastore/src/Storage/MetastoreStorageInterface.php @@ -26,7 +26,7 @@ public function count(bool $unpublished = FALSE): int; * @param bool $published * Whether to retrieve the published revision of the metadata. * - * @return string|HydratableInterface + * @return string|null * The data or null if no data could be retrieved. * * @throws \Drupal\metastore\Exception\MissingObjectException @@ -114,7 +114,7 @@ public function remove(string $id); /** * Store. * - * @param string|HydratableInterface $data + * @param string $data * The data to be stored. * @param string $id * The identifier for the data. If the act of storing generates the @@ -126,7 +126,7 @@ public function remove(string $id); * @throws \Exception * Issues storing the data. */ - public function store($data, string $id = NULL): string; + public function store(string $data, string $id = NULL): string; /** * Retrieve by hash. diff --git a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php index ffeb868e73..ac1fa8313a 100644 --- a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php +++ b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php @@ -275,7 +275,7 @@ public function testChangeMediaType() { }'; $data = json_decode($json); $referencer->reference($data); - $storedResource = DataResource::hydrate($container_chain->getStoredInput('resource')[0]); + $storedResource = DataResource::createFromRecord(json_decode($container_chain->getStoredInput('resource')[0])); // A new resource should have been stored, with the mimetype set to text/csv $this->assertEquals('text/csv', $storedResource->getMimeType()); } diff --git a/phpunit.xml b/phpunit.xml index 3ec3332fd3..bdeb809091 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -26,31 +26,22 @@ . - tests/src/Unit modules/common/tests/src/Unit - modules/metastore/tests/src/Unit - modules/metastore/modules/metastore/tests/src/Unit modules/metastore/modules/metastore_search/tests/src/Unit - modules/metastore/modules/metastore_admin/tests/src/Unit modules/datastore/tests/src/Unit modules/datastore/modules/datastore_mysql_import/tests/src/Unit modules/frontend/tests/src/Unit modules/dkan_js_frontend/tests/src modules/harvest/tests/src/Unit - modules/harvest/modules/harvest_dashboard/tests/src/Unit modules/json_form_widget/tests/src/Unit modules/common/tests/src/Functional modules/metastore/tests/src/Functional - modules/metastore/modules/metastore/tests/src/Functional modules/metastore/modules/metastore_search/tests/src/Functional modules/metastore/modules/metastore_admin/tests/src/Functional modules/datastore/tests/src/Functional - modules/frontend/tests/src/Functional modules/dkan_js_frontend/tests/src - modules/harvest/tests/src/Functional - modules/harvest/modules/harvest_dashboard/tests/src/Functional tests/src/Functional diff --git a/tests/src/Functional/DatasetTest.php b/tests/src/Functional/DatasetTest.php index 80b33b9d4d..35eac47e8c 100644 --- a/tests/src/Functional/DatasetTest.php +++ b/tests/src/Functional/DatasetTest.php @@ -106,6 +106,7 @@ public function testResourcePurgeDraft() { $this->getMetastore()->publish('dataset', $id_1); $this->storeDatasetRunQueues($id_1, '1.3', ['1.csv', '5.csv'], 'put'); + /** @var \Drupal\common\DatasetInfo $datasetInfo */ $datasetInfo = \Drupal::service('dkan.common.dataset_info'); $info = $datasetInfo->gather($id_1); $this->assertStringEndsWith('1.csv', $info['latest_revision']['distributions'][0]['file_path']);