Feature - Ability to define different PermissionRegistrar

[illambo]

This PR replaces the PR #2618 refactors some code of the PermissionRegistrar and traits.

The goal is the possibility of being able to instantiate multiple PermissionRegistrar to isolate and use the library conveniently on multiple database at the same time (with possible different configurations without having to modify the runtime configuration).

Overview code changes :

• Improvement PermissionRegistrar
  • refactoring initialization
  • configuration isolation
  • strengthening of properties type declarations
• Allowed ability to use custom PermissionRegistrar for models and traits
• Allowed ability to use custom PermissionRegistrar in commands
• Allowed ability to use custom PermissionRegistrar for some methods in helpers
• Add tests cases for multiple schema and adapted some tests

The configuration is given (and isolated for the potentially extensible parts) as input during the initialization phase of the PermissionRegistrar. It is also possible to overwrite the PermissionRegistrar used for models and traits, in this way you can use the library in a "straight" way on multiple schemes without having to change the configuration at runtime (see the battery of tests for greater clarity).

Currently I have not brought the entire configuration internally so as not to apply too many changes, but only the bare minimum that I have found currently implemented.

This approach is similar to the one also present in other libraries, for example see spatie/laravel-medialibrary (see provider and trait).

A possible breaking change is in the setPermissionClass and setRoleClass methods of the PermissionRegistrar where the global configuration is no longer updated as it is irrelevant. I kept the binding of the contract optional although in my opinion it should be managed on the application side according to the business logic of the case).

I am aware that it is quite substantial as a PR and I probably missed something around because I don't know the entire library in depth, but I still wanted to propose it as a starting point for a future release (or in any case to consider its purpose).

Thanks

[illambo]

Hi, any news about that? Thanks

[drbyte]

Still reviewing this. As you say, it's quite substantial.

[illambo]

Hi, were you able to evaluate for a moment the underlying reasons for this PR? What do you think? Do you think it's a valid approach for this or a next major release? In enterprise environments, in my opinion, it often happens that we deal with multiple db schema with libraries like this (or medialibrary, ..). Thanks

[drbyte]

Hi, were you able to evaluate for a moment the underlying reasons for this PR?

I believe I understand your objective, and am willing to consider it for future release. There's a lot to consider when a PR introduces a lot more code to manage :D

[drbyte]

Would it still work if instead of calling $permissionRegistrar = static::getPermissionRegistrar(); in multiple functions, it were to be set as a property once when the trait is instantiated?

[illambo]

Would it still work if instead of calling $permissionRegistrar = static::getPermissionRegistrar(); in multiple functions, it were to be set as a property once when the trait is instantiated?

I haven't tried, it could work but you need to try, you could do some tests so you can verify your thinking and check with the battery of tests. I know I have adapted a lot of code to "the need" so I ask you to check and if necessary adapt everything as best as possible in terms of style or code for the library. This PR can also simply be a starting point / suggestion for conversion in this sense in a future release (so feel free to close it if it is too premature), in my opinion we could also take advantage of the opportunity to strengthen the method signature (php >= 8.1 or 8.2) and the cleaning of logic and code.

Thanks for the feedback.