Please fix your codestyle issues.

I don't have a good feeling with this proposal. So far I haven't been able to fully work through the proposal completely, but based on the complexity, this is not something that could be part of 3.4.x. This will need time to get reviewed and tested, especially combined with the issue that we don't want to fill our session with megabytes of access check data.

we don't want to fill our session with megabytes of access check data.

I really do not understand what do you mean, where do you see caching in session?

Sorry, my bad. We only cache the viewlevels and for some stupid reason the groups a user belongs to, but in a place where they are never used in the access checking.

Which
makes
this
even
worse.

The ACL system was designed to cache its results per user in the respective session in order to prevent a query for each and every single authorise() call. It meant that per session, you only had to do those checks (theoretically) once. I was absolutely sure that we had this in our code at some point, but I couldn't find it, at least back to February 2010. At that point, the session table had a varchar field for the data with a limit of 256kb afaik. That is why we discussed having to have a garbage collector for those authorise checks or in other words: A limit on how much we cache. Now it seems that Louis/Andrew dropped that again when they saw that this meant that access changes would not propagate unless the users sessions are wiped once.

And I'm wondering why the system is slow like a turtle...

sigh Ok, so lets introduce caching. That would have to be done in JUser, since JUser is already stored in the session. I would say that we should not cache more than 500 permissions per session.

In any case, that is WAY out of scope of 3.4.0.

ok, I understand your point

but
my pull have different approach,
if we will be able to check multiple assets at once (means do only one db request instead of 100 separate requests for each asset) then we also can win a lot, even without caching

my pull do not change existing JAsset::check() logic, so I see no problem to have it somewhere 3.4.x ... but it more depend from test results and feedbacks of course

My problem is, that we will be stuck with your method for the rest of the 3.x series. At the same time it is a REALLY fragile part of Joomla. So I want to make sure that we are really doing the right thing here.

@Hackwar about caching, as you already noticed it can lead to huge session size, that will make session save/update slower (especially on huge site), that even without such caching not so fast ..

maybe this was a reason why this not implemented/removed

so from my side I do not see huge performance win compared to "check multiple assets at once"

I added caching in #6053. On my system its about 10-20% faster on the second load than before. There is quite a large performance gain here, because for example module editing will be requested identically on all pages and that means 0 extra queries per page if the caching is used. As you can see, I also limited the caching to 250 elements, exactly because of the session-size-issue. That means about 10-15kb of extra session data in worst case. We might have to tweak that further in the future.

With your implementation I'm hesitant, because, as I said, the code that we add has to be "bullet-proof", since we are stuck with it until 4.0 at least. If this turns out to have a huge negative effect, we need to be able to revert that change.

Considering your implementation:

• checkMultiple is a bad name for this. You could use batchCheck or simply extend JAccess::check() and let it accept an array as second parameter instead of just a string and depending on that behave differently.
• No code should have to call JAccess directly, this should all go through JUser, so you would either need to extend JUser::authorise the way I described above or provide a method that wraps this in JUser accordingly.
• The frontend editing feature is a frontend-only feature and thus should not be moved into JModuleHelper. (It is also wrong in the renderer and we should find a different solution for this, but that is another issue) Please leave the feature in the modules renderer.
• Please don't use nested ternary operators.
• Looking at the query to retrieve the rules, I don't know if that one works correctly. I don't fully understand it, but to me it looks like you will first retrieve all specific module rules, then all com_modules rules, then the core.edit rule or whatever and the module rules will override each other pretty randomly. As far as I understand it, they are not ordered by path-per-ID-to-check and then from leaf to root, but all leafs are returned first, then all parents of those leafs, etc. and they will overwrite each other. Will have to look into that one a bit further however.

On my system its about 10-20% faster on the second load than before

main word on your system

thing that I am afraid:

it is Joomla! 3.3, home page, clear installation,
on the hosting where the database optimized to read, but slow on write/update, and there request to "session update" always slower on the page (in range between 40-90ms and even more)

so each additional caching in the session is a killing feature

Looking at the query to retrieve the rules, I don't know if that one works correctly

I would not do pull request without testing