[
https://issues.apache.org/jira/browse/OAK-8234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16817666#comment-16817666
]
angela edited comment on OAK-8234 at 4/15/19 9:05 AM:
------------------------------------------------------
[~stillalex], while the intended change makes a lot of sense to me, i don't
particularly like the delegatee approach with yet another redirection. in
particular since it creates a lot of changes to the {{PermissionProviderImpl}}
that might result in subtle regressions. Is there a reason for not making the
distinction between administrative subjects and regular subjects right in the
{{AuthorizationConfigurationImpl}}? that would place that fundamental
difference between set of principals that are subjected to permission
evaluation vs those that are not prominently in the flow. also for
administrative principals the whole {{Mount}} handling is IMO redundant and
which is not taken into account in the patch as far as i saw. wdyt?
regarding the failing test: please don't mark tests as ignored. if we decide
that the test is not relevant for administrative principals, we should move it
to a test for non-administrative principals... since a lot of tests for the
current {{PermissionProvider}} are writting for an admin user, we might
refactor the tests a well if administrative subjects get another instance of
{{PermissionProvider}}. the overall branch coverage of the permission package
is 86% today and we should IMO improve that before making such a major
refactoring.
was (Author: anchela):
[~stillalex], while the intended change makes a lot of sense to me, i don't
particularly like the delegatee approach with yet another redirection. in
particular since it creates a lot of changes to the {{PermissionProviderImpl}}
that might result in subtle regressions. Is there a reason for not making the
distinction between administrative subjects and regular subjects right in the
{{AuthorizationConfigurationImpl}}? that would place that fundamental
difference between set of principals that are subjected to permission
evaluation vs those that are not prominently in the flow. also for
administrative principals the whole {{Mount}} handling is IMO redundant and
which is not taken into account in the patch as far as i saw. wdyt?
> Reduce object allocation in PermissionProviderImpl for admin sessions
> ---------------------------------------------------------------------
>
> Key: OAK-8234
> URL: https://issues.apache.org/jira/browse/OAK-8234
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core, security
> Reporter: Alex Deparvu
> Assignee: Alex Deparvu
> Priority: Major
>
> There was already a lot of work done here to improve efficiency of admin
> sessions, but we can still do better by simply not creating many of the
> objects needed as parameters for the {{CompiledPermissions}} which will be
> ignored anyway in the case of admin sessions.
> Ex: There are a lot of immutable trees (created via
> {{PermissionUtil.getReadOnlyTree(tree, immutableRoot)}}) that are not used
> for the evaluation.
> Thanks to [~mreutegg] for spotting this one!
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
