Skip to content
Draft
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: 3 additions & 0 deletions modules/metastore/config/schema/metastore.schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ metastore.settings:
data_dictionary_sitewide:
type: string
label: 'Sitewide Dictionary ID'
disable_json_validation:
type: boolean
label: 'Disable JSON Validation'
csv_headers_mode:
type: string
label: 'CSV Headers Mode'
Expand Down
21 changes: 21 additions & 0 deletions modules/metastore/metastore.module
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,24 @@ function metastore_theme() {
],
];
}

/**
* Implements hook_requirements().
*/
function metastore_requirements($phase) {
$requirements = [];

if ($phase === 'runtime') {
$config = \Drupal::config('metastore.settings');
if ($config->get('disable_json_validation')) {
$requirements['metastore_validation_disabled'] = [
'title' => t('DKAN data validation is disabled'),
'value' => t('Validation is disabled'),
'description' => t('Data validation is currently disabled in DKAN Metastore Settings. This may allow invalid data to be saved. It is recommended to enable validation as soon as possible.'),
'severity' => REQUIREMENT_WARNING,
];
}
}

return $requirements;
}
1 change: 1 addition & 0 deletions modules/metastore/metastore.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ services:
- '@dkan.metastore.storage'
- '@dkan.metastore.metastore_item_factory'
- '@cache_tags.invalidator'
- '@config.factory'
- '@module_handler'

dkan.metastore.url_generator:
Expand Down
20 changes: 20 additions & 0 deletions modules/metastore/src/Form/DkanDataSettingsForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
$form['html_allowed_html'] = $this->getHtmlAllowedHtml($config);
$form['property_list'] = $this->getPropertyList($config);
$form['orphan'] = $this->getOrphanCleanupFields($config);
$form['disable_json_validation'] = $this->getValidationCheckbox($config);

return parent::buildForm($form, $form_state);
}
Expand Down Expand Up @@ -120,6 +121,24 @@ private function getRedirectCheckbox(Config $config) {
];
}

/**
* Builds the checkbox form element for disabling json validation.
*
* @param \Drupal\Core\Config\Config $config
* The metastore settings configuration.
*
* @return array
* The form element array.
*/
private function getValidationCheckbox(Config $config) {
return [
'#type' => 'checkbox',
'#title' => $this->t('Temporarily disable JSON validation.'),
'#default_value' => $config->get('disable_json_validation') ?? FALSE,
'#description' => $this->t('If you are having a problem due to invalid data and are unable to remove it, use this option to temporarily disable JSON validation and allow the invalid data to be removed. Use with caution, and consider fixing the underlying data issue as soon as possible.'),
];
}

/**
* Builds the text box for allowed HTML elements.
*
Expand Down Expand Up @@ -229,6 +248,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
->set('html_allowed_html', $form_state->getValue('html_allowed_html'))
->set('orphan.delete', $form_state->getValue('delete'))
->set('orphan.retain_for', $form_state->getValue('retain_for'))
->set('disable_json_validation', $form_state->getValue('disable_json_validation'))
->save();

// Rebuild routes, without clearing all caches.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Drupal\metastore\Plugin\Validation\Constraint;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\metastore\ValidMetadataFactory;
use OpisErrorPresenter\Implementation\MessageFormatterFactory;
Expand All @@ -17,6 +18,13 @@
*/
class ProperJsonValidator extends ConstraintValidator implements ContainerInjectionInterface {

/**
* Module configuration.
*
* @var \Drupal\Core\Config\ConfigFactoryInterface
*/
protected $metastoreConfig;

/**
* Service dkan.metastore.valid_metadata.
*
Expand All @@ -34,10 +42,13 @@ class ProperJsonValidator extends ConstraintValidator implements ContainerInject
/**
* ProperJsonValidator constructor.
*
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* Config factory service.
* @param \Drupal\metastore\ValidMetadataFactory $valid_metadata_factory
* Service dkan.metastore.valid_metadata.
*/
public function __construct(ValidMetadataFactory $valid_metadata_factory) {
public function __construct(ConfigFactoryInterface $config_factory, ValidMetadataFactory $valid_metadata_factory) {
$this->metastoreConfig = $config_factory->get('metastore.settings');
$this->validMetadataFactory = $valid_metadata_factory;
$this->presenter = new ValidationErrorPresenter(
new PresentedValidationErrorFactory(
Expand All @@ -51,7 +62,8 @@ public function __construct(ValidMetadataFactory $valid_metadata_factory) {
*/
public static function create(ContainerInterface $container) {
return new static(
$container->get('dkan.metastore.valid_metadata')
$container->get('config.factory'),
$container->get('dkan.metastore.valid_metadata'),
);
}

Expand All @@ -60,8 +72,22 @@ public static function create(ContainerInterface $container) {
*
* {@inheritdoc}
*/
public function validate($items, Constraint $constraint) {
public function validate($items, Constraint $constraint): void {
// Bypass validation if the setting to disable it is enabled.
if (!$this->metastoreConfig->get('disable_json_validation')) {
$this->validateItems($items);
}
}

/**
* Validates items and adds violations if needed.
*
* @param object|mixed $items
* Items to validate.
*/
protected function validateItems($items): void {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not really want to break this out into a separate function, but Qlty had opinions. Adding the one if wrapper brought it up to 6 layers deep.

too many layers

$schema_id = $this->getSchemaIdFromEntity($items);
// Loop through items, validating each and collecting errors.
foreach ($items as $item) {
$errors = $this->doValidate($schema_id, $item);
if (!empty($errors)) {
Expand Down
43 changes: 35 additions & 8 deletions modules/metastore/src/Reference/ReferenceLookup.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Contracts\FactoryInterface;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\metastore\Factory\MetastoreItemFactoryInterface;
use Drupal\metastore\ReferenceLookupInterface;
Expand Down Expand Up @@ -35,25 +36,43 @@ class ReferenceLookup implements ReferenceLookupInterface {
*/
private CacheTagsInvalidatorInterface $invalidator;

/**
* Module configuration.
*
* @var \Drupal\Core\Config\ConfigInterface
*/
protected $metastoreConfig;

/**
* Module handler service.
*/
private ModuleHandlerInterface $moduleHandler;

/**
* Module Handler service.
* ReferenceLookup constructor.
*
* @var \Drupal\Core\Extension\ModuleHandlerInterface
* @param \Contracts\FactoryInterface $metastoreStorage
* The metastore storage service.
* @param \Drupal\metastore\Factory\MetastoreItemFactoryInterface $metastoreItemFactory
* The metastore item factory service.
* @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $invalidator
* The cache tags invalidator service.
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* The config factory service.
* @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler
* The module handler service.
*/
public function __construct(
FactoryInterface $metastoreStorage,
MetastoreItemFactoryInterface $metastoreItemFactory,
CacheTagsInvalidatorInterface $invalidator,
ModuleHandlerInterface $moduleHandler
ConfigFactoryInterface $config_factory,
ModuleHandlerInterface $moduleHandler,
) {
$this->metastoreStorage = $metastoreStorage;
$this->metastoreItemFactory = $metastoreItemFactory;
$this->invalidator = $invalidator;
$this->metastoreConfig = $config_factory->get('metastore.settings');
$this->moduleHandler = $moduleHandler;
}

Expand Down Expand Up @@ -142,14 +161,22 @@ protected function decodeJsonMetadata(string $json): array {
$identifier = $metadata->identifier;
// Get raw metadata using identifier.
$metadata = $this->metastoreItemFactory->getInstance($identifier)->getRawMetadata();
// Validate JSON against legacy schema.
$validation_result = RootedJsonData::validate(json_encode($metadata), $legacy_schema);
// If the JSON metadata matches the legacy schema, extract the content of
// the "data" property.
if ($validation_result->isValid()) {
// Make sure validation is not disabled.
if (!$this->metastoreConfig->get('disable_json_validation')) {
// Validate JSON against legacy schema.
$validation_result = RootedJsonData::validate(json_encode($metadata), $legacy_schema);
// If the JSON metadata matches the legacy schema, extract the content of
// the "data" property.
if ($validation_result->isValid()) {
$metadata = $metadata->data;
}
}
else {
// Do no validation, move it all along.
$metadata = $metadata->data;
}


return [$identifier, $metadata];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,24 @@
namespace Drupal\Tests\metastore\Unit\Plugin\Validation\Constraint;

use Drupal\metastore\Plugin\Validation\Constraint\ProperJsonValidator;
use Drupal\metastore\ValidMetadataFactory;
use Drupal\metastore\SchemaRetriever;
use Drupal\metastore\ValidMetadataFactory;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Validator\Constraints\Count;
use Symfony\Component\Validator\Context\ExecutionContext;
use PHPUnit\Framework\TestCase;

/**
* Class.
*/
class ProperJsonValidatorTest extends TestCase {
class ProperJsonValidatorTest extends UnitTestCase {

/**
* The config factory used for testing.
*
* @var \Drupal\Core\Config\ConfigFactoryInterface
*/
protected $configFactory;

/**
* The schema retriever used for testing.
Expand Down Expand Up @@ -57,14 +64,22 @@ protected function setUp(): void {
->getMock();
$this->validMetadataFactory->method('getSchemaRetriever')->willReturn($this->schemaRetriever);

$this->configFactory = $this->getConfigFactoryStub([
'metastore.settings' => [
'disable_json_validation' => FALSE,
],
]);

$this->container = $this->getMockBuilder(ContainerInterface::class)
->onlyMethods(['get'])
->disableOriginalConstructor()
->getMockForAbstractClass();

$this->container->method('get')
->with('dkan.metastore.valid_metadata')
->willReturn($this->validMetadataFactory);
->willReturnMap([
['config.factory', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, $this->configFactory],
['dkan.metastore.valid_metadata', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, $this->validMetadataFactory],
]);

$this->context = $this->getMockBuilder(ExecutionContext::class)
->onlyMethods(["addViolation"])
Expand Down