Create models for ALM APIs

[pkbullock]

Category

• Feature request

Describe the feature

• Add the ability to call the ALM APIs as descirbed in the article: https://docs.microsoft.com/en-us/sharepoint/dev/apis/alm-api-for-spfx-add-ins?tabs=sprest
• Unit Tests for calling the APIs
• If there are special requirements or permissions with this model, add to the documentation

Contributing

We encourage and welcome contributions for the PnP Core SDK, if you are interested, please comment below to engage with the team. If you want to learn about how to get started, please refer to this guide: https://pnp.github.io/pnpcore/articles/contributor/readme.html

The team is constantly improving the guidance on supporting contributors, if you have any feedback or need help please feel free to start a discussion.

[s-KaiNet]

I would like to deep dive into this awesome library internals and implement this issue if there are no objections.

[s-KaiNet]

Should I continue working on the ALM API? If yes, could you assign the issue to me, so that we have an indication that the work is in progress?

[jansenbe]

Hi @s-KaiNet , sorry for not seeing your message earlier on. We would love to get your contributions in this area!! Feel free to reach out if you've questions, for now I've assigned this to you.

[jansenbe]

You've probably seen this one already: in PnP Framework we did built ALM support a long time ago (https://github.com/pnp/pnpframework/blob/dev/src/lib/PnP.Framework/ALM/AppManager.cs). Might help with the PnP Core SDK version of it. Once the PnP Core SDK one is fully baked we can replace the PnP Framework internals with what PnP Core SDK offers.

Also please be aware that we're in the process of adding a PnP.Core.Admin library and we need to decide where to add the ALM features: PnP.Core, PnP.Core.Admin or somehow spread across both (PnP.Core.Admin can use PnP.Core but PnP.Core cannot use PnP.Core.Admin classes and methods).

As said in previous comment, happy to get into a call and discuss. Thanks for helping out @s-KaiNet 🚀💪

[s-KaiNet]

Hi @jansenbe, great, thank you!

Yeah, I saw AppManager from PnP.Framework. In PnP.Framework it's implemented as a separate class. My initial idea for the ALM API was to leverage the new PnP Core SDK model and implement it as a regular IDataModel class. Meaning that you can do smth like this:

pnpContext.Web.TenantAppCatalog.Apps.Add(...);
pnpContext.Web.TenantAppCatalog.Apps.GetById(...);
pnpContext.Web.TenantAppCatalog.Apps.GetById(...).Deploy();
//etc

// or
pnpContext.Web.SiteCollectionAppCatalog.Apps.Add(...);
//etc

However with your information regarding Admin library it seems that my initial idea was not so good.
Moving ALM API into the Admin package has below pros over having it in the Core:

• programming model will be very similar to the "old" AppManager - easier to migrate for developers and easier to migrate for PnP.Framework
• this API from my assumption is not so frequently used, thus adding new properties to the Web interface, i.e. pnpContext.Web.TenantAppCatalog isn't very efficient and it's more natural to have them in a separate package. Semantically apps management is also more about admin tasks, rather than common things.
• it looks like I need at least one method from Admin package - GetTenantAppCatalogUriAsync. Since we cannot reference Admin from Core, ALM should be in the Admin

Taking all above into the account, my view regarding the ALM API implementation is below:

1. Implement it in the Admin package.
2. Update PnPContextExtensions (from Admin package) with a method GetAppManager() which returns and instance of the AppManager
3. The AppManager should be implemented as class with public interface (like SharePointAdmin for example)
4. The public API for AppManager should be somewhat similar to the original AppManager (for easier migrations).

I'm happy to see any comments or suggestions regarding my view. If you're agree with above statements I will continue the implementation.

[jansenbe]

Tagging @PaoloPia and @pkbullock to ensure they've seen this as well.

I do like your planned approach @s-KaiNet , feels indeed better to have the ALM features hosted in PnP.Core.Admin. Imagine there's an IApp interface then that interface can have methods like DeployAsync and Deploy etc. The fact the functionality will be added in the Admin library also does not mean that you can't use the IDataModel concepts as they're used in PnP.Core...but I agree that that approach is not necessarily adding a lot of value in this case.

I do like the model of adding a new extension method GetAppManager(), today I've been adding features under GetSharePointAdmin() and GetMicrosoft365Admin() but adding everything there would not help with clarity. So I'm considering breaking up the current model and also introduce a GetSiteManager() extension method and corresponding interface. The app catalog method we have today can then move to the IAppManager interface. We can overtime (v2?) even consider moving the Teams app features to here to give developers a central place to manage the apps in their Microsoft365 tenant.

Thanks again for taking up this task, happy to get in a meeting with you to further discuss if needed.

[jansenbe]

@s-KaiNet : I've pushed a change that introduced ISiteCollectionManager and IAppManager with their respective methods, implementations and extension methods. Best to ensure you've updated from the latest before starting your development. Splitting up by feature area is much better compared to adding everything under ISharePointAdmin, thanks for making us aware of that 👍

[s-KaiNet]

@jansenbe I have a question regarding the API itself. I will really appreciate it if you or other team members, who are deeply involved in the library development, are able to provide their input.

In the official documentation, we have only a few methods available for the TenantAppCatalog endpoint (Add, SyncSolutionToTeams). However, when looking at the metadata it appears that currently there are some more methods.

Another story is that there is also a site collection app catalog endpoint. Here we have a lot fewer methods than in the TenantAppCatalog.

• should we implement additional methods currently available in the metadata for the TenantAppCatalog endpoint?
• if the answer above is yes, then what would be the best approach to distinguish TenantAppCatalog and SiteCollectionAppCatalog calls? Because some methods will be available only for tenant catalog (DownloadTeamsSolution, SolutionContainsTeamsComponent..., some others) and it might not be clear for a developer, that you can call them only for tenant app catalog.
  One option would be just document it in the method signature and that's it.
  Another option would be using two different managers (with common codebase) - GetTenantAppManager() (with all the methods from metadata for TenantAppCatalog) and GetSiteCollectionAppManager() for SiteCollectionCatalog. In that case it will be very clear for a developer which catalog they work with.
  Alternateveliy, we can still use one AppManager class, however in that case we should use AppCatalogScope enum for some methods. Also, when we use one AppManager it's not very clear what to do when a developer tries to call a method from tenant app catalog using site collection context (silently switch context? throw an exception?).

[jansenbe]

Hi @s-KaiNet ,

About the site collection versus tenant app catalog differences: having an ITenantAppManager and ISiteCollectionAppManager interfaces that inherit from a common IAppManager interface + two extension methods like you mentioned is the best solution as it offers clarity for the developer. The lesser ambiguity the SDK allows the easier it's to use.

Regarding the extra methods on the tenant app catalog: there are some useful methods and properties that can be implemented, if you can spare the extra cycles then feel free to add these. This can also happen later in a second phase if that works better for you.

[s-KaiNet]

@jansenbe I'm 90% done with the ALM API. I have one question though.

In the PnP.Framework we have SyncToTeams method. It seems that the corresponding SP REST endpoint doesn't work at all with Bearer tokens and OAuth authentication. I created an issue at sp-dev-docs repo where you can find a bit more details. Also, I found a thread in m365 cli repo where they also rejected this API endpoint since it doesn't work with the JWT access tokens.

Taking into account all the above, I assume that we shouldn't include all SyncToTeams* methods in the PnP Core, since they simply don't work. At least for the time being until the issue isn't fixed on the SharePoint REST API side. I can just comment out the code and put a link to the sp-dev-docs issue. Do you agree?

[jansenbe]

@s-KaiNet , yes that's fine. API calls that cannot work with an access token cannot be used in PnP Core SDK and should be fixed by Microsoft.