[Improvement] Refactor query for performance gain

[mattamon]

Resolves #700 - at least it is a start.

Refactoring the query is one part.
We should also introduce an index like the following, but we can do this in the core itself with a migration.
See pimcore/pimcore#17834

CREATE INDEX idx_users_workspaces ON users_workspaces_object (userId, cpath, list);
CREATE INDEX idx_users_workspaces ON users_workspaces_asset (userId, cpath, list);

[fashxp]

Sadly, this query does not deliver the correct results 😞.

Take the following permission configuration which must not show /Product Data/Accessories/rims:

The old query looks something like this for userid 20

SELECT id, path, `key` FROM objects WHERE path = '/Product Data/Accessories/' AND `key` = 'rims' AND
((select list from users_workspaces_object where userId in (20) and LOCATE(CONCAT("/Product Data/Accessories/","rims"),cpath)=1
    ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
OR
(select list from users_workspaces_object where userId in (20) and LOCATE(cpath,CONCAT("/Product Data/Accessories/","rims"))=1
    ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
); 

... and returns zero results - which is correct.

The new query looks something like this

SELECT id, path, `key` FROM objects WHERE path = '/Product Data/Accessories/' AND `key` = 'rims' AND
    EXISTS (
        SELECT 1
        FROM users_workspaces_object
        WHERE userId IN (20)
          AND (
            LOCATE(CONCAT("/Product Data/Accessories/","rims"), cpath) = 1
                OR LOCATE(cpath, CONCAT("/Product Data/Accessories/","rims")) = 1
            )
          AND list = 1
    );

... and returns results - which is wrong.

The reason why while testing in the UI it doesn't show a result is that line - which double checks permissions with calling isAllowed for each element.

This in turns becomes a problem for example with following permission configuration, which should show one line in the grid:

With the old query, it works as expected.
With the new query it doesn't - because the query delivers a whole page of false-positives (which are filtered out) so the true-positive doesn't make it into the result.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[mattamon]

Sadly, this query does not deliver the correct results 😞.

Take the following permission configuration which must not show /Product Data/Accessories/rims:

The old query looks something like this for userid 20

SELECT id, path, `key` FROM objects WHERE path = '/Product Data/Accessories/' AND `key` = 'rims' AND
((select list from users_workspaces_object where userId in (20) and LOCATE(CONCAT("/Product Data/Accessories/","rims"),cpath)=1
    ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
OR
(select list from users_workspaces_object where userId in (20) and LOCATE(cpath,CONCAT("/Product Data/Accessories/","rims"))=1
    ORDER BY LENGTH(cpath) DESC LIMIT 1)=1
); 

... and returns zero results - which is correct.

The new query looks something like this

SELECT id, path, `key` FROM objects WHERE path = '/Product Data/Accessories/' AND `key` = 'rims' AND
    EXISTS (
        SELECT 1
        FROM users_workspaces_object
        WHERE userId IN (20)
          AND (
            LOCATE(CONCAT("/Product Data/Accessories/","rims"), cpath) = 1
                OR LOCATE(cpath, CONCAT("/Product Data/Accessories/","rims")) = 1
            )
          AND list = 1
    );

... and returns results - which is wrong.

The reason why while testing in the UI it doesn't show a result is that line - which double checks permissions with calling isAllowed for each element.

This in turns becomes a problem for example with following permission configuration, which should show one line in the grid:

With the old query, it works as expected. With the new query it doesn't - because the query delivers a whole page of false-positives (which are filtered out) so the true-positive doesn't make it into the result.

Valid points. I used now only some micro-optimization since LIKE seems to performe better than Locate.

At least the results look now better to me but I am not sure what big of an impact it has.
For my tests we are not far off the admin time.

[cancan101]

@mattamon @kingjia90 was pimcore/pimcore#17834 ever re-PRed against a feature branch?

[mattamon]

@cancan101 yes :) pimcore/pimcore#17847