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.

Sign up for GitHub

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

Jump to bottom

[Bug]: Fix GridView Performance and results #767

Merged
merged 2 commits into from
Nov 25, 2024
Merged

[Bug]: Fix GridView Performance and results #767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ac73f6
fix grid permission check performance
kingjia90 Nov 25, 2024
9952524
use the search one
kingjia90 Nov 25, 2024
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
16 changes: 10 additions & 6 deletions src/Controller/Admin/Asset/AssetController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1673,12 +1673,16 @@ public function getFolderContentPreviewAction(Request $request, EventDispatcherI

if (!$this->getAdminUser()->isAdmin()) {
$userIds = $this->getAdminUser()->getRoles();
$userIds[] = $this->getAdminUser()->getId();
$conditionFilters[] = ' (
(select list from users_workspaces_asset where userId in (' . implode(',', $userIds) . ') and LOCATE(CONCAT(`path`, filename),cpath)=1 ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
OR
(select list from users_workspaces_asset where userId in (' . implode(',', $userIds) . ') and LOCATE(cpath,CONCAT(`path`, filename))=1 ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
)';
$currentUserId = $this->getAdminUser()->getId();
$userIds[] = $currentUserId;

$inheritedPermission = $folder->getDao()->isInheritingPermission('list', $userIds);

$anyAllowedRowOrChildren = 'EXISTS(SELECT list FROM users_workspaces_asset uwa WHERE userId IN (' . implode(',', $userIds) . ') AND list=1 AND LOCATE(CONCAT(`path`,filename),cpath)=1 AND
NOT EXISTS(SELECT list FROM users_workspaces_asset WHERE userId =' . $currentUserId . ' AND list=0 AND cpath = uwa.cpath))';
$isDisallowedCurrentRow = 'EXISTS(SELECT list FROM users_workspaces_asset WHERE userId IN (' . implode(',', $userIds) . ') AND cid = id AND list=0)';

$conditionFilters[] = 'IF(' . $anyAllowedRowOrChildren . ',1,IF(' . $inheritedPermission . ', ' . $isDisallowedCurrentRow . ' = 0, 0)) = 1';
}

$condition = implode(' AND ', $conditionFilters);
Expand Down
90 changes: 71 additions & 19 deletions src/Helper/GridHelperService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@
use PhpOffice\PhpSpreadsheet\Writer\Exception;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx;
use Pimcore\Bundle\AdminBundle\Event\AdminEvents;
use Pimcore\Db;
use Pimcore\Logger;
use Pimcore\Model;
use Pimcore\Model\DataObject;
use Pimcore\Model\DataObject\ClassDefinition;
use Pimcore\Model\DataObject\Objectbrick;
use Pimcore\Model\Element\Service;
use Pimcore\Model\User;
use Symfony\Component\EventDispatcher\GenericEvent;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
Expand Down Expand Up @@ -186,7 +188,7 @@ public function getFilterCondition(string $filterJson, ClassDefinition $class, ?
$conditionPartsFilters = [];

if ($filterJson) {
$db = \Pimcore\Db::get();
$db = Db::get();
$filters = json_decode($filterJson, true);

foreach ($filters as $filter) {
Expand Down Expand Up @@ -297,7 +299,7 @@ public function getFilterCondition(string $filterJson, ClassDefinition $class, ?
$brickFilterField = $field->getName();
}

$db = \Pimcore\Db::get();
$db = Db::get();

if ($isLocalized) {
$brickPrefix = $db->quoteIdentifier($brickType . '_localized') . '.';
Expand Down Expand Up @@ -427,7 +429,7 @@ public function addGridFeatureJoins(DataObject\Listing\Concrete $list, array $fe
$featureAndSlugFilters,
$me
) {
$db = \Pimcore\Db::get();
$db = Db::get();

$alreadyJoined = [];

Expand Down Expand Up @@ -477,7 +479,7 @@ public function addSlugJoins(DataObject\Listing\Concrete $list, array $slugJoins
$featureAndSlugFilters,
$me
) {
$db = \Pimcore\Db::get();
$db = Db::get();

$alreadyJoined = [];

Expand Down Expand Up @@ -614,13 +616,7 @@ public function prepareListingForGrid(array $requestParams, string $requestedLan
}

if (!$adminUser->isAdmin()) {
$userIds = $adminUser->getRoles();
$userIds[] = $adminUser->getId();
$conditionFilters[] = ' (
(select list from users_workspaces_object where userId in (' . implode(',', $userIds) . ') and LOCATE(CONCAT(`path`,`key`),cpath)=1 ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
OR
(select list from users_workspaces_object where userId in (' . implode(',', $userIds) . ') and LOCATE(cpath,CONCAT(`path`,`key`))=1 ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
)';
$conditionFilters[] = $this->getPermittedPathsByUser('object', $adminUser);
}

$featureJoins = [];
Expand Down Expand Up @@ -732,7 +728,7 @@ public function prepareListingForGrid(array $requestParams, string $requestedLan

public function prepareAssetListingForGrid(array $allParams, User $adminUser): Model\Asset\Listing
{
$db = \Pimcore\Db::get();
$db = Db::get();
$folder = Model\Asset::getById((int) $allParams['folderId']);

$start = 0;
Expand Down Expand Up @@ -851,13 +847,7 @@ public function prepareAssetListingForGrid(array $allParams, User $adminUser): M
}

if (!$adminUser->isAdmin()) {
$userIds = $adminUser->getRoles();
$userIds[] = $adminUser->getId();
$conditionFilters[] = ' (
(select list from users_workspaces_asset where userId in (' . implode(',', $userIds) . ') and LOCATE(CONCAT(`path`, filename),cpath)=1 ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
OR
(select list from users_workspaces_asset where userId in (' . implode(',', $userIds) . ') and LOCATE(cpath,CONCAT(`path`, filename))=1 ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
)';
$conditionFilters[] = $this->getPermittedPathsByUser('asset', $adminUser);
}

//filtering for tags
Expand Down Expand Up @@ -932,4 +922,66 @@ public function createXlsxExportFile(FilesystemOperator $storage, string $fileHa

return $response;
}


/**
*
*
* @internal
*/
protected function getPermittedPathsByUser(string $type, User $user): string
{
$db = Db::get();

$allowedTypes = [];

if ($user->isAllowed($type . 's')) { //the permissions are just plural
$elementPaths = Service::findForbiddenPaths($type, $user);

$forbiddenPathSql = [];
$allowedPathSql = [];
foreach ($elementPaths['forbidden'] as $forbiddenPath => $allowedPaths) {
$exceptions = '';
$folderSuffix = '';
if ($allowedPaths) {
$exceptionsConcat = implode("%' OR `path` LIKE '", $allowedPaths);
$exceptions = " OR (`path` LIKE '" . $exceptionsConcat . "%')";
$folderSuffix = '/'; //if allowed children are found, the current folder is listable but its content is still blocked, can easily done by adding a trailing slash
}
$forbiddenPathSql[] = ' (`path` NOT LIKE ' . $db->quote($forbiddenPath . $folderSuffix . '%') . $exceptions . ') ';
}
foreach ($elementPaths['allowed'] as $allowedPaths) {
$allowedPathSql[] = ' `path` LIKE ' . $db->quote($allowedPaths . '%');
}

// this is to avoid query error when implode is empty.
// the result would be like `(((path1 OR path2) AND (not_path3 AND not_path4)))`
$forbiddenAndAllowedSql = '(';

if ($allowedPathSql || $forbiddenPathSql) {
$forbiddenAndAllowedSql .= '(';
$forbiddenAndAllowedSql .= $allowedPathSql ? '( ' . implode(' OR ', $allowedPathSql) . ' )' : '';

if ($forbiddenPathSql) {
//if $allowedPathSql "implosion" is present, we need `AND` in between
$forbiddenAndAllowedSql .= $allowedPathSql ? ' AND ' : '';
$forbiddenAndAllowedSql .= implode(' AND ', $forbiddenPathSql);
}
$forbiddenAndAllowedSql .= ' )';
}

$forbiddenAndAllowedSql.= ' )';

$allowedTypes[] = $forbiddenAndAllowedSql;
}

//if allowedTypes is still empty after getting the workspaces, it means that there are no any main permissions set
// by setting a `false` condition in the query makes sure that nothing would be displayed.
if (!$allowedTypes) {
$allowedTypes = ['false'];
}

return '('.implode(' OR ', $allowedTypes) .')';
}

}
Loading