Added IPermissionDefinitionService for Abstractions Project

[GerardSmit]

Fixes #5840

Summary

This PR adds IPermissionDefinitionService which is an abstraction layer for PermissionController.

Instead of returning an ArrayList in the GetPermissions*-methods, I've made the return type IEnumerable<IPermissionInfo>; this reduces allocations (ArrayList instance and two arrays). For all these methods I've created an Enumerable method, that's being used by the current implementation and interface implementation; this is so we don't have duplicated code.

I've only created an interface for this class. All calls to the PermissionController haven't been refactored.

TODO

• Merge IPermissionInfoBase and IPermissionInfo into a single interface IPermissionInfo

[valadas]

Nothing sticks out to me. Just no in love with IPermissionInfoBase name which makes it look like a base class. What about we use IPermission and IPermissionInfo instead of IPermissionInfo and IPermissionInfoBase ? Just a thought...

[GerardSmit]

The only downside to IPermission is that it's an already existing interface in .NET Framework: System.Security.IPermission.

We can also merge the two interfaces to a single interface IPermissionInfo, since the current implementations of *PermissionInfo all extends PermissionInfoBase and PermissionInfo:

[valadas]

Sounds good. Hey what tool you use for that graph ?

[GerardSmit]

I used ReSharper to create that diagram.

What I did:

1. Left click the interface name and click "Derrived Symbols":
   
2. Click on the first icon "Show on Diagram":
   
3. Then I changed the view to "Business Logic":
   

[bdukes]

Thanks!

[bdukes]

I agree, let's merge IPermissionInfo and IPermissionBase

[GerardSmit]

Sadly, I couldn't merge IPermissionInfo and IPermissionInfoBase, since I misunderstood PermissionInfo.

PermissionInfo is from dbo.Permissions, which are the definitions.
PermissionInfoBase is the base for dbo.TabPermission, dbo.ModulePermission etc. they are the actual permission to the object they belong to (e.g. Tab or Module) with their definition.

I've renamed IPermissionInfo to IPermissionDefinition to prevent confusion, and IPermissionInfoBase to IPermissionInfo. This only applies to the abstractions, the actual implementation still have the same name (class PortalInfo and PortalInfoBase).

Is this OK?

[bdukes]

Sounds great to me, thanks!

[valadas]

Nice, given this is the resource manager and not part of any distributed nuget package I am not worried about the breaking changes in this.

[GerardSmit]

I've checked /Admin/File-Management if the permissions are being loaded and saved. Everything works as expected 🎉

[GerardSmit]

I'm still thinking about the interface name. If we're gonna make abstractions for PermissionProvider, should this be done in the IPermissionService as well?

Another solution is to rename this interface to IPermissionDefinitionService, and when we make abstractions for PermissionProvider we can call the interface name IPermissionService.

[bdukes]

I'm still thinking about the interface name. If we're gonna make abstractions for PermissionProvider, should this be done in the IPermissionService as well?

Another solution is to rename this interface to IPermissionDefinitionService, and when we make abstractions for PermissionProvider we can call the interface name IPermissionService.

I think IPermissionDefinitionService is a better name, since every member deals with IPermissionDefinitionInfo.

[GerardSmit]

There are still 2 comments open: #5841 (comment), #5841 (comment); other than that everything is done.

[bdukes]

Thanks @GerardSmit, this is looking great. The @dnnsoftware/approvers will meet next Tuesday, so we may not get a definitive answer on those open questions until then. We appreciate your contribution and your patience.

[bdukes]

@dnnsoftware/approvers I've rebased on develop, updated the deprecations to 9.13.0, added deprecations for the properties which we're changing casing on, and also extracted IFolderPermissionInfo as part of adjusting usages of the deprecated properties. This should be good to merge.

[valadas]

Awesome, great work!