Add ISkinService for Abstractions Project

[GerardSmit]

Fixes #5837

Summary

This PR adds ISkinService which is an abstraction layer for SkinController.

Things to validate

1. I've renamed properties with ID to Id.
   Only DotNetNuke.Abstractions.Users.IUserInfo uses ID (AffiliateID, PortalID, UserID), the rest uses Id.
2. I've moved 2 enums to the abstractions; I've added [TypeForwardTo] so extensions don't have to recompile.
3. I didn't mark the static methods as [Obsolete] and I haven't modified the calls to SkinController.
4. I've added the service as a scoped service; just in case the service will use scoped services in the future.

[valadas]

I have 2 small concernes but I am not sure if relevant, just something to double-check...

[bdukes]

Thanks! We appreciate your work, this is a nice and clean PR. Hopefully we can quickly resolve our couple of questions and get it included.

[GerardSmit]

I've changed the abstraction a little bit, in my eyes it makes more sense this way.

SkinPackageType

I've created a new enum named "SkinPackageType", this can be either Skin or Container.
You can use this enum to get or update everything in the interface. For example:

Instead of having a property "RootSkin" or "RootContainer", you call: GetFolderName(SkinPackageType.Skin).
Instead of having 4 methods (e.g. "GetDefaultPortalSkin"), you call: GetDefaultSkinSrc(SkinPackageType.Skin, SkinType.Edit)
Instead of changing the skin with the folder name, you call: SetSkin(SkinPackageType.Skin, ...)

Skin in SkinPackageInfo

I also did see that there is a SkinInfo, but it's never used. I changed SkinPackageInfo.Skins to a list of SkinInfo. If we ever add more details in the table dbo.Skins, we can add these to this class. Before the Skins was a Dictionary with ID as key and SkinSrc as value. This means it's a breaking change, but it's already targeting v10. Is this OK?

Because of the abstractions, I cannot implement IList<ISkinInfo> because in the implementation this is a List<SkinInfo>. I've created a new wrapper class called AbstractionList<TInterface, TImplementation> which casts all the interfaces to the implementation.

In the abstractions I've added IObjectList<T>, which is used in SkinPackageInfo.Skins. This interface has an extra method called T AddNew() which creates a new instance of the implementation, adds it to the list and returns the instance. Because you don't have SkinInfo in the abstractions; you can never create it. Now you can do the following:

ISkinPackageInfo skinPackageInfo; // Instance of the Skin Package

ISkinInfo newSkin = skinPackageInfo.Skins.AddNew(); // New skin

Naming

I've also changed the naming a little bit, for example GetSkins returns an enumerable with the skins in a folder; not from the database. I've changed the name to GetSkinsInFolder.

---

This means the abstraction is quite different than the implementation now, but it's way clearer what every method does, just by the name and arguments. If there is any feedback I like to hear it.

[bdukes]

@dnnsoftware/approvers I've rebased on develop, should be good to merge.

[bdukes]

Thanks so much!

[bdukes]

Just thinking about this PR, should we call the new abstraction IThemeService?

[valadas]

Just thinking about this PR, should we call the new abstraction IThemeService?

I don't have a strong preference. I would love it but then a lot of other code uses Skin terminology so not sure if it would add or remove confusion :)

[bdukes]

I think we can merge this PR as-is. I opened a new PR to discuss the potential naming change.

[GerardSmit]

Note, the SkinController only updates the SkinsList (skinPackage is ISkinPackageInfo here):

Dnn.Platform/DNN Platform/Library/UI/Skins/SkinController.cs

Lines 641 to 656 in 2d85973

	/// <inheritdoc cref="ISkinService.UpdateSkinPackage" />
	private static void UpdateSkinPackage(ISkinPackageInfo skinPackage)
	{
	DataProvider.Instance().UpdateSkinPackage(
	skinPackage.SkinPackageId,
	skinPackage.PackageId,
	skinPackage.PortalId,
	skinPackage.SkinName,
	SkinUtils.ToDatabaseName(skinPackage.SkinType),
	UserController.Instance.GetCurrentUserInfo().UserID);
	EventLogController.Instance.AddLog(skinPackage, PortalController.Instance.GetCurrentPortalSettings(), UserController.Instance.GetCurrentUserInfo().UserID, string.Empty, EventLogController.EventLogType.SKINPACKAGE_UPDATED);
	foreach (var skin in skinPackage.Skins)
	{
	UpdateSkin(skin.SkinId, skin.SkinSrc);
	}
	}

Even though the property is deprecated, it's read-only now.

[bdukes]

I'm not sure I understand. Are you saying that UpdateSkinPackage is wrong to use skinPackage.Skins? Or that the ISkinPackageInfo.Skins property is fundamentally broken? Or that we're missing an update to keep the two collection in sync?

[GerardSmit]

We currently have two properties:

Skins = Dictionary<int, string>
SkinsList = List<ISkinInfo>

If you change the SkinsList and call UpdateSkinPackage the skins will be updated.
However, if you change the Skins and call UpdateSkinPackage the skins will not be updated, which does work in the current version.

Or that we're missing an update to keep the two collection in sync?

Correct.

[david-poindexter]

I propose we move this PR to a 9.14.0 milestone. @dnnsoftware/approvers ?

[david-poindexter]

Per Technology meeting today, we agreed on a 9.14.0 milestone for this PR. Thanks!