Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 0.16
    • core
    • None

    Description

      As suggested by jukka on the mailing list we may consider removing the tree type from the ImmutableTree and ImmutableRoot and just keep it inside the permission evaluation code.

      pro: simplify ImmutableTree

      con: for PermissionProvider#isGranted and PermissionProvider#hasPrivilege the treepermission object is not present and retrieving the type always will walk up the hierarchy to retrieve the type of the parent with the associated performance impact.

      Attachments

        Activity

          proposed patch

          angela Angela Schreiber added a comment - proposed patch

          The patch doesn't cleanly apply for me. However generally +1 for such an approach.

          mduerig Michael DĂĽrig added a comment - The patch doesn't cleanly apply for me. However generally +1 for such an approach.

          i am just a bit hesitating because of the drawback. while i usually claim that applications should not check for permissions manually, i can't prevent them from doing so and i would obviously not want to introduce any extra performance problems.

          angela Angela Schreiber added a comment - i am just a bit hesitating because of the drawback. while i usually claim that applications should not check for permissions manually, i can't prevent them from doing so and i would obviously not want to introduce any extra performance problems.
          jukkaz Jukka Zitting added a comment -

          con: for PermissionProvider#isGranted and PermissionProvider#hasPrivilege the treepermission object is not present and retrieving the type always will walk up the hierarchy to retrieve the type of the parent with the associated performance impact.

          The isGranted() and hasPrivilege() method are hardly ever performance-critical, as their main purpose is to provide guidance to a human user interface about which operations are available. So I wouldn't worry too much about extra performance overhead here.

          jukkaz Jukka Zitting added a comment - con: for PermissionProvider#isGranted and PermissionProvider#hasPrivilege the treepermission object is not present and retrieving the type always will walk up the hierarchy to retrieve the type of the parent with the associated performance impact. The isGranted() and hasPrivilege() method are hardly ever performance-critical, as their main purpose is to provide guidance to a human user interface about which operations are available. So I wouldn't worry too much about extra performance overhead here.

          well... i would wish you were right
          anyway, we can go that way and see if this really causes troubles.

          angela Angela Schreiber added a comment - well... i would wish you were right anyway, we can go that way and see if this really causes troubles.
          stillalex Alex Deparvu added a comment -

          bulk close for the 0.16 release

          stillalex Alex Deparvu added a comment - bulk close for the 0.16 release

          People

            angela Angela Schreiber
            angela Angela Schreiber
            Votes:
            0 Vote for this issue
            Watchers:
            Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment