Skip to content

Commit

Permalink
chore: Add static cache to WorkspaceHelper::checkPermission. There do…
Browse files Browse the repository at this point in the history
…esn't seem a real need to check the same element over and over again in the same request.
  • Loading branch information
das-peter committed Aug 20, 2024
1 parent fba3737 commit d5787b3
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 10 deletions.
13 changes: 13 additions & 0 deletions doc/10_GraphQL/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,17 @@ The mockup support can be enabled and configured with a configuration entry like
pimcore_data_hub:
graphql:
mockup_element_support_enabled: true
```

## Permission Check Cache

By default only a single permission check per request is executed.
This is different from previous versions in which the event
`pimcore.datahub.graphql.permission.preCheck` was fired multiple times during the execution.

The new approach limits the performance impact of the permission handling - but can be disabled using following setting:
```yml
pimcore_data_hub:
graphql:
disable_permission_check_cache: true
```
1 change: 1 addition & 0 deletions src/Controller/WebserviceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ public function webonyxAction(
if (isset($datahubConfig['graphql']) && isset($datahubConfig['graphql']['not_allowed_policy'])) {
PimcoreDataHubBundle::setNotAllowedPolicy($datahubConfig['graphql']['not_allowed_policy']);
}
$context['disable_permission_check_cache'] = !empty($datahubConfig['graphql']['disable_permission_check_cache']);
$context['mockup_element_support_enabled'] = !empty($datahubConfig['graphql']['mockup_element_support_enabled']);

$validators = null;
Expand Down
1 change: 1 addition & 0 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public function getConfigTreeBuilder()
->booleanNode('output_cache_enabled')->info('enables output cache for graphql responses. It is disabled by default')->defaultValue(false)->end()
->integerNode('output_cache_lifetime')->info('output cache in seconds. Default is 30 seconds')->defaultValue(30)->end()
->booleanNode('mockup_element_support_enabled')->info('Enable support of mockup elements as used by Ecommerce Framework Bundle IndexService. You have to use your own Mockup class(es) that implement DataHubs ElementMockupInterface')->defaultValue(false)->end()
->booleanNode('disable_permission_check_cache')->info('Disables the permission check static cache. Use only if you have very specific cases in which the permission can change _within_ the same request!')->defaultValue(false)->end()
->booleanNode('allow_introspection')->info('enables introspection for graphql. It is enabled by default')->defaultValue(true)->end()
->booleanNode('run_subrequest_per_query')->info('If enabled a Symfony Sub-Requet is triggered for each query in a multi-query request.')->defaultValue(false)->end()
->end()
Expand Down
36 changes: 26 additions & 10 deletions src/WorkspaceHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public static function deleteConfiguration(Configuration $config)
*
* @throws NotAllowedException
*/
public static function checkPermission($element, $type, string $elementType = '')
public static function checkPermission($element, $type, string $elementType = '', bool $skipCache = false)
{
$context = RuntimeCache::get('datahub_context');
/** @var Configuration $configuration */
Expand All @@ -235,7 +235,26 @@ public static function checkPermission($element, $type, string $elementType = ''
if ($configuration->skipPermisssionCheck()) {
return true;
}

// This can be called multiple times with the same element - use static
// cache to avoid multiple execution in favor of performance.
// @TODO Is there really a reason for the configuration?
if (!$elementType) {
if ($element instanceof ElementMockupInterface) {
$elementType = $element->getElementType();
} else {
$elementType = Service::getElementType($element);
}
}
$cid = $element->getId() . ':' . $type . ':' . $elementType ;
if (!$skipCache && empty($context['disable_permission_check_cache'])) {
if (!RuntimeCache::isRegistered(__METHOD__)) {
RuntimeCache::set(__METHOD__, []);
}
$permissionCache = RuntimeCache::get(__METHOD__);
if (isset($permissionCache[$cid])) {
return $permissionCache[$cid];
}
}
$event = new PermissionEvent($element, $type);
/** @var EventDispatcher $eventDispatcher */
$eventDispatcher = \Pimcore::getContainer()->get('event_dispatcher');
Expand All @@ -245,18 +264,15 @@ public static function checkPermission($element, $type, string $elementType = ''
}
// we can allow nullable elements e.g. linked Assets where the asset itself was removed
if (!$element) {
return true;
$permissionCache[$cid] = true;
RuntimeCache::set(__METHOD__, $permissionCache);
return $permissionCache[$cid];
}

$isAllowed = self::isAllowed($element, $configuration, $type, $elementType);
$permissionCache[$cid] = $isAllowed;
RuntimeCache::set(__METHOD__, $permissionCache);
if (!$isAllowed && PimcoreDataHubBundle::getNotAllowedPolicy() === PimcoreDataHubBundle::NOT_ALLOWED_POLICY_EXCEPTION) {
if (!$elementType) {
if ($element instanceof ElementMockupInterface) {
$elementType = $element->getElementType();
} else {
$elementType = Service::getElementType($element);
}
}
// Could be dealing with mock objects that can't be loaded due
// to stale index so be extra cautions when using.
try {
Expand Down

0 comments on commit d5787b3

Please sign in to comment.