Important Fix to Ensure Application Order of Permissions - Take II

Dear SpaceManiac:

My apologies about the double pull request. I have a habit of running a source formatter to keep code clean. Unfortunately, that has a tendency to mask which lines were actually changed during revision comparison. I have ensured that this is not the case in this second pull request.

I previously fixed this problem for version 1.4 and pushed it to KrinsDeath's, given he was the more current maintainer of a working copy. Attempts to contact you on this matter previously were unsuccessful. However, he had no interest in accepting it. Hopefully, given your renewed interest in maintaining this plugin, this pull request will find its way into the an official release.

This pull request includes a critical update to ensure permissions are applied in the order in which they are declared rather than an internal order of a HashMap.  Regular HashMaps make no guarantee that its elements are arranged according to their insertion order - LinkedHashMaps do.

Furthermore, the order in which permissions are gathered has been changed to reflect the natural order of precedence, rather than a calculated one. Permissions are gathered from the most ancestral group first, overridden by its world-specific settings, working its way down the family tree back to the user. This is **particularly** important when dealing with parent permissions.

Two case studies: 1) Assume the following permission declaration for a user or a group.

    mchat.*: false        # Ultra paranoid, explicitly turn everything off
    mchat.list: true      # Turn on /list command
    mchat.me: true        # Turn on /me command

Given the current implementation, the declaration order is not guaranteed. In real situations, the order can become

    mchat.list: true
    mchat.me: true
    mchat.*: false

as you iterate over a permission collection while adding them to a player's permission set. It would not have the expected result. Thus, it is important to use an ordered hash such as LinkedHashMaps to ensure that iteration order is the same as insertion order. This is how SnakeYaml preserves declaration order.

2) Assume the following configuration

    users:
        Notch:
            permissions:
                mchat.list: true
            groups:
                - default

    groups:
        default:
            permissions:
                - mchat.*: false

The expected result is that all mchat features are disabled by the group definition, but that its list function is enabled for Notch. However, because the current version populates user permissions first and then collects group permissions the order of application (assuming that they are ordered correctly) is

    mchat.list: true
    mchat.*: false

instead of

    mchat.*: false
    mchat.list: true

If it weren't for parent permissions, this approach would be sufficient. However, even though specific checks are made to see if a permission is already declared before overwriting it, it does not work for parent permissions. Your plugin has no a priori knowledge of what permissions are included in a parent permissions; those are only expanded once they are added to the permissible structures.

The only thing I am not certain of is whether I was working with the most current source. Per BukkitDev, PermissionsBukkit is at version 1.6, yet the POM on GitHub shows version 1.5. That is what these modifications are based on. If so, you should still be able to migrate these critical changes over easily. There are additional comments in the source.

Best regards,

Frelling