adobe / asset-share-commons

A modern, open-source asset share reference implementation built on Adobe Experience Manager (AEM)
https://opensource.adobe.com/asset-share-commons/
Apache License 2.0
88 stars 107 forks source link

Asset Type Computation Is Inconsistent #544

Open tkkbz opened 4 years ago

tkkbz commented 4 years ago

Describe the bug The asset type computed by AssetTypeImpl is inconsistent due to the way it's implemented. The extension is extracted as the last part of the dc:format property, some examples:

This value is then checked against the mimetype lookup provided by Adobe: /libs/dam/gui/content/assets/jcr:content/mimeTypeLookup In short: the extension is checked against an array of mimetypes for the childnodes of this specific lookup node to determine the asset type.

There is however a type in AEM 6.4 (checked on 6.4.7) where every last value of the mimetype property of these childnodes has a ; dangling.

64-lookupnode

Since the UIHelper in the code splits on ,, the last value to compare the extension to will never match. Applied to the examples:

Since this value is used to resolve the details page for a specific asset, this results in different behaviour for 2 asset of same mimetype, which should have asset type: video:

In AEM 6.5 (checked 6.5.6), this dangling ; is removed however, meaning the MP4 assets will also match MULTIMEDIA in the lookup node, meaning the MP4 assets will not resolve to the details/video.html Asset Share details page anymore.

65-lookupnode

Environment

Current Impact

Expected behavior 2 assets (video's) should resolve to the same details page and asset type should be determined in a consistent way. I believe the type selection should not depend on the AEM libs nodes:

  1. not all extensions are included in the lookup node, eg xxx.ppt where the extension according to the code is vnd.ms-powerpoint. Hence this asset type is determined by the if/else cases. This makes Asset Type selection inconsistent.
  2. creates a dependency on an AEM libs node while the behaviour for resolving asset share details pages based on an asset type is Asset Share Commons specific (any change to the lookup node might have undesired impact on Asset Share).

The Asset Type should be only be determined by the if/else cases to result in expected behaviour. Extra mapping by OSGi config could allow more complex/exception Asset Type computation

godanny86 commented 4 years ago

@thmsdbts I agree. Some background, we were trying to keep Asset Share Commons UI consistent with AEM Assets UI. We would welcome a PR that improves this. Another approach is also to extend the Asset Type computed property: https://adobe-marketing-cloud.github.io/asset-share-commons/pages/development/guide/ -> Extend Computed Properties

davidjgonzalez commented 4 years ago

You can also create a new AssetDetailsResolver [1] impl (examples: [2]), which will automatically show up as an option on the Search Page's configuration [3]) and you can use that to pick the details page. The Asset Details page resolution was designed to be pluggable since we knew different people would want different approaches (breaking it out by "file type" is very generic use case).

[1] https://github.com/Adobe-Marketing-Cloud/asset-share-commons/blob/develop/core/src/main/java/com/adobe/aem/commons/assetshare/configuration/AssetDetailsResolver.java [2] https://github.com/Adobe-Marketing-Cloud/asset-share-commons/blob/develop/core/src/main/java/com/adobe/aem/commons/assetshare/configuration/impl/selectors/AssetTypeSelectorImpl.java [3] https://adobe-marketing-cloud.github.io/asset-share-commons/pages/search/search-page/