Netcentric / accesscontroltool

Rights and roles management for AEM made easy
Eclipse Public License 1.0
147 stars 92 forks source link

Accesscontroltool includes deprecated sling.settings package #624

Open royteeuwen opened 2 years ago

royteeuwen commented 2 years ago

When including the access control tool in an AEM as a cloud Service setup, I get the following warning from the aem analyser:

[WARNING] [region-deprecated-api] biz.netcentric.cq.tools.accesscontroltool:accesscontroltool-bundle:3.0.4: Usage of deprecated package found : org.apache.sling.settings : AEM as a Cloud Service does not support run modes or file system access at runtime. Deprecated since 2020-10-05 (com.my-site.all:1.0.0-SNAPSHOT|Netcentric:accesscontroltool-package:3.0.4)

It would be nice to remove that part of the code for the cloud classifier (or refactor it out completely)

ghenzler commented 2 years ago

We support runmodes in folder names for AC Tool yaml configuration in the same way as AEM/Sling supports runmode-specific OSGi configs: https://github.com/Netcentric/accesscontroltool/blob/dbb2c88cf4523df756ae32c9832eedb003022e18/docs/Configuration.md#run-modes

Therefore currently the package org.apache.sling.settings is also used in the cloud.

To get rid of the usage we would (and probably will) have to configure an OSGi dummy service for each cloud supported runmode token and implement our https://github.com/Netcentric/accesscontroltool/blob/develop/accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/slingsettings/ExtendedSlingSettingsServiceImpl.java that way.

kwin commented 2 years ago

According to https://experienceleague.adobe.com/docs/experience-manager-cloud-service/content/release-notes/deprecated-apis.html?lang=en this should have been removed end of 2021 already, but even ACS AEM Commons is still using that and noone recommended a usable alternative I would for now just ignore this, until this has been solved in ACS AEM Commons (https://github.com/Adobe-Consulting-Services/acs-aem-commons/issues/2476)

royteeuwen commented 2 years ago

Yea I hate it that they say "its going to be removed" but they didn't just do it, lets just get it over with so that we don't have to stall everything but still the analyser keeps spitting warnings.

Anyways, would it be a good alternative to make two builds? a cloud and non cloud build? In the cloud build you could use another implementation than the Impl there is now for non-cloud, and in the cloud build you just exclude the existing impl?