Adobe-Consulting-Services / acs-aem-commons

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

Client Versioning is Not working in AEM 6.5.8 #2577

Closed Prakash2288 closed 3 years ago

Prakash2288 commented 3 years ago

Required Information

Expected Behavior

ClientLibs versioning should work in AEM 6.5.8

Actual Behavior

ClientLibs versioning is not working in AEM 6.5.8

### Steps to Reproduce 1) Create a vanilla instance of AEM 6.5.8 2) Install acs-aem-commons-4.8.4 3) Enable the client lib versioning by installing below package vslingfloderclientlib.zip 4) Verify the client lib is getting versionize by inspecting page from the path "/content/we-retail/us/en/men.html?wcmmode=disabled". But it is not getting versionize. Refer below screen shot image_2021_04_26T06_23_40_510Z

Note: Clientlib versioning is working fine with AEM service pack 6.5.5 but when we upgrade service pack to 6.5.8 it is not working as expected. Only css file is getting versioned but not the js files. we tried with acs-common-5.0.4 and aem service pack 6.5.8 but with acs common 5.0.4 both the css and js file is not getting versionized.

Links

http://localhost:4505/etc.clientlibs/weretail/clientlibs/clientlib-base.css http://localhost:4505/etc.clientlibs/weretail/clientlibs/clientlib-base.js

YegorKozlov commented 3 years ago

I have nagging doubts it has to do to https://github.com/Adobe-Consulting-Services/acs-aem-commons/pull/2533

With this PR the code resolves client libs before trying to get it with HtmlLibraryManager and it does not seem to work with proxied paths (/etc.clientlibs)

The culprit is VersionedClientlibsTransformerFactory#resolvePath

    private String resolvePath(LibraryType libraryType, String libraryPath, SlingHttpServletRequest request) {
        Resource libraryResource = request.getResourceResolver().resolve(request, libraryPath);
        if (libraryResource != null && !(libraryResource instanceof NonExistingResource)) {
            return libraryResource.getPath();
        }
        return resolvePathIfProxied(libraryType, libraryPath, request.getResourceResolver());
    }

If libraryPath is proxied., e.g. /etc.clientlibs/my-app/clientlibs/clientlib-base then request.getResourceResolver().resolve(request, libraryPath) will resolve it to /etc and the method will return "/etc" which is wrong.

I guess the resolve logic should not apply to proxied paths:

    private String resolvePath(LibraryType libraryType, String libraryPath, SlingHttpServletRequest request) {
        if (!libraryPath.startsWith(PROXY_PREFIX)) { // don't try to resolve proxies paths, e.g. /etc.clientlibs/my-app/clientlibs/clientlib-base
            Resource libraryResource = request.getResourceResolver().resolve(request, libraryPath);
            if (libraryResource != null && !(libraryResource instanceof NonExistingResource)) {
                return libraryResource.getPath();
            }
        }
        return resolvePathIfProxied(libraryType, libraryPath, request.getResourceResolver());
    }
kwin commented 3 years ago

This is fixed by #2581

irenebijo commented 3 years ago

Hi Kwin, I am on ACS version 5.0.4 and AEM 6.5.8 . I am still facing the issue. But I have taken the ACS commons 5.0.4 prior to 15 day before the fix is pushed. https://github.com/Adobe-Consulting-Services/acs-aem-commons/releases

Can you please share the acs commons 5.0.4 link, which has this fix.

Thanks, Irene.

kwin commented 3 years ago

This is fixed in the upcoming 5.0.6 (always check the milestone on closed issues)

irenebijo commented 3 years ago

Hi Kwin,

Can you please let us know, where can I see the milestone for the version 5.0.6. It works on Some AEM instances. Is there any recommendation to make it workable with acs 5.0.4 version?

Thanks, Irene

neelupadhyay01 commented 3 years ago

@kwin

This is regarding issues reported by @irenebijo .For us also we are using AMS where in non prod environment below ACS commons package is deployed.

Current non prod version where we are testing the changes-

aem-service-pkg-6.5.8.zip acs-aem-commons-content-5.0.4.zip core.wcm.components.all-2.14.0.zip

Existing prod package-

aem-service-pkg-6.5.6.zip
acs-aem-commons-ui.content-4.8.6
core.wcm.components.config-2.11.0.zip

For us we have tested 60 different applications pages,JS and css which comes under one org group.We haven't seen this issue in non prod environment.Do you think I should go ahead and push the change to production.

I also saw your note that issue is fixed with 5.0.6.Not sure if we need to stick on 5.0.4 or wait for 5.0.6.

Please advise.