search

Adobe-Consulting-Services / acs-aem-commons

http://adobe-consulting-services.github.io/acs-aem-commons/
Apache License 2.0
453 stars 598 forks source link

Content Classification Violations #1550

Closed kwin closed 5 years ago

kwin commented 5 years ago

Required Information

Expected Behavior

ACS Commons should fully comply with the Content Classification rules being outlined at https://helpx.adobe.com/experience-manager/6-4/sites/deploying/using/sustainable-upgrades.html

Actual Behavior

If you verify the according health-check at /libs/granite/operations/content/healthreports/healthreport.html/system/sling/monitoring/mbeans/org/apache/sling/healthcheck/HealthCheck/SlingContentHealthCheck you get a lot of warning due to ACS Commons violating content classification.

screenshot 2018-11-09 at 17 55 18

Steps to Reproduce

Just open /libs/granite/operations/content/healthreports/healthreport.html/system/sling/monitoring/mbeans/org/apache/sling/healthcheck/HealthCheck/SlingContentHealthCheck on AEM 6.4+ and check the warnings.

Links

https://helpx.adobe.com/experience-manager/6-4/sites/deploying/using/sustainable-upgrades.html

justinedelson commented 5 years ago

@kwin I think it would be better to create specific issues for each violation. When I review the list, I see a lot of things which appear to be invalid. The first one in your screenshot is a good example of this.

kwin commented 5 years ago

Well, the first ones are issued because https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/master/content/src/main/content/jcr_root/apps/acs-commons/components/workflow/watson-audio-transcription/.content.xml inherit (via sling:resourceSuperType) from cq/workflow/components/model/external_process which has mixin granite:FinalArea. That means according to the Adobe documentation

Defines a node as final. Nodes classified as final cannot be overlaid or inherited. Final nodes can be used directly via sling:resourceType. Subnodes under final node are considered internal by default

You are clearly violating that guideline by inheriting from that resource type. So I don't think this is a false positive.

kwin commented 5 years ago

On the other hand https://helpx.adobe.com/experience-manager/6-4/sites/developing/using/workflows-customizing-extending.html#CreatingCustomWorkflowStepComponents clearly states

To inherit from one of the (existing) base step components, add the following property to the cq:Component node: Name: sling:resourceSuperType Type: String Value: One of the following paths that resolves to a base component: cq/workflow/components/model/process cq/workflow/components/model/participant cq/workflow/components/model/dynamic_participant

justinedelson commented 5 years ago

Again, we should have this discussion in an "violation"-specific issue. But I'm quite confident that this is a false positive since otherwise there is no way to create an External Workflow Process. This is a documentation gap.

kwin commented 5 years ago

The workflow-related supertypes have been acknowledged meanwhile as false-positive and the fix is being tracked in CQ-4249019 (fixed with AEM 6.5). The only other types being reported as violations are

  1. inheriting from granite/ui/components/foundation/form/select and
  2. inheriting from granite/ui/components/foundation/form/pathbrowser and
  3. inheriting from cq/cloudserviceconfigs/templates/configpage

About those I am not too sure. @justinedelson Do you know whether those are false-positives as well or whether those are just no longer allowed to be inherited.

justinedelson commented 5 years ago

I believe #3 is a false positive. Using this supertype is explicitly documented at https://helpx.adobe.com/experience-manager/6-4/sites/developing/using/extending-cloud-config.html

I'm not sure about #1 and #2.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

kwin commented 5 years ago

Just checked again on AEM 6.5 (/libs/granite/operations/content/healthreports/healthreport.html/system/sling/monitoring/mbeans/org/apache/sling/healthcheck/HealthCheck/SlingContentHealthCheck) and with acs-aem-commons 4.0.0 no content classification issues are reported any longer!

kwin commented 4 years ago

Outstanding issues tracked in #1511