avoid multiple includes

wcm-io · Sep 9, 2024 · 794c4e7 · 794c4e7
1 parent e9f8e40
commit 794c4e7
Show file tree

Hide file tree

Showing 7 changed files with 161 additions and 4 deletions.
diff --git a/changes.xml b/changes.xml
@@ -23,7 +23,13 @@
     xsi:schemaLocation="http://maven.apache.org/changes/1.0.0 http://maven.apache.org/plugins/maven-changes-plugin/xsd/changes-1.0.0.xsd">
   <body>
 
-    <release version="1.3.2" date="not released">
+    <release version="1.4.0" date="not released">
+      <action type="update" dev="sseifert">
+        Do not include JS/CSS client libraries twice if the same library is requested multiple times within one request.
+      </action>
+      <action type="update" dev="sseifert">
+        Ensure request context path is added to client library URLs.
+      </action>
       <action type="update" dev="sseifert">
         Switch to AEM 6.5.17 as minimum version.
       </action>

diff --git a/src/main/java/io/wcm/wcm/ui/clientlibs/components/CSSInclude.java b/src/main/java/io/wcm/wcm/ui/clientlibs/components/CSSInclude.java
@@ -51,6 +51,8 @@ public class CSSInclude {
   private static final Set<String> REL_ALLOWED_VALUES = Set.of(
       "prefetch", "preload");
 
+  @SlingObject
+  private SlingHttpServletRequest request;
   @SlingObject
   private ResourceResolver resourceResolver;
   @OSGiService
@@ -109,13 +111,20 @@ private void activate() {
    */
   private @NotNull String buildIncludeString(@NotNull List<String> libraryPaths, @NotNull Map<String, String> attrs,
       @NotNull Map<String, String> customAttrs) {
+    RequestIncludedLibraries includedLibraries = new RequestIncludedLibraries(request);
     StringBuilder markup = new StringBuilder();
     for (String libraryPath : libraryPaths) {
+      // ignore libraries that are already included
+      if (includedLibraries.isInlucded(libraryPath)) {
+        continue;
+      }
       HtmlTagBuilder builder = new HtmlTagBuilder("link", false, xssApi);
       builder.setAttrs(attrs);
       builder.setAttrs(customAttrs);
-      builder.setAttr("href", libraryPath);
+      builder.setAttr("href", request.getContextPath() + libraryPath);
       markup.append(builder.build());
+      // mark library as included
+      includedLibraries.storeIncluded(libraryPath);
     }
     return markup.toString();
   }

diff --git a/src/main/java/io/wcm/wcm/ui/clientlibs/components/IncludeUtil.java b/src/main/java/io/wcm/wcm/ui/clientlibs/components/IncludeUtil.java
@@ -97,7 +97,7 @@ else if (categories != null && categories.getClass().isArray()) {
       path = "/etc.clientlibs" + path.substring(5);
     }
     else if (resourceResolver.getResource(library.getPath()) == null) {
-      // current render resourcer resolver has no access to the client library - ignore it
+      // current render resource resolver has no access to the client library - ignore it
       path = null;
     }
     return path;

diff --git a/src/main/java/io/wcm/wcm/ui/clientlibs/components/JSInclude.java b/src/main/java/io/wcm/wcm/ui/clientlibs/components/JSInclude.java
@@ -57,6 +57,8 @@ public class JSInclude {
   private static final Set<String> TYPE_ALLOWED_VALUES = Set.of(
       "text/javascript", "module");
 
+  @SlingObject
+  private SlingHttpServletRequest request;
   @SlingObject
   private ResourceResolver resourceResolver;
   @OSGiService
@@ -145,13 +147,20 @@ private void activate() {
    */
   private @NotNull String buildIncludeString(@NotNull List<String> libraryPaths, @NotNull Map<String, String> attrs,
       @NotNull Map<String, String> customAttrs) {
+    RequestIncludedLibraries includedLibraries = new RequestIncludedLibraries(request);
     StringBuilder markup = new StringBuilder();
     for (String libraryPath : libraryPaths) {
+      // ignore libraries that are already included
+      if (includedLibraries.isInlucded(libraryPath)) {
+        continue;
+      }
       HtmlTagBuilder builder = new HtmlTagBuilder("script", true, xssApi);
       builder.setAttrs(attrs);
       builder.setAttrs(customAttrs);
-      builder.setAttr("src", libraryPath);
+      builder.setAttr("src", request.getContextPath() + libraryPath);
       markup.append(builder.build());
+      // mark library as included
+      includedLibraries.storeIncluded(libraryPath);
     }
     return markup.toString();
   }

diff --git a/src/main/java/io/wcm/wcm/ui/clientlibs/components/RequestIncludedLibraries.java b/src/main/java/io/wcm/wcm/ui/clientlibs/components/RequestIncludedLibraries.java
@@ -0,0 +1,74 @@
+/*
+ * #%L
+ * wcm.io
+ * %%
+ * Copyright (C) 2024 wcm.io
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
+package io.wcm.wcm.ui.clientlibs.components;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+
+import com.adobe.granite.ui.clientlibs.HtmlLibraryManager;
+import com.drew.lang.annotations.NotNull;
+
+class RequestIncludedLibraries {
+
+  /**
+   * Request attributes that is also used by Granite UI clientlib manager to store the (raw) library
+   * paths that are already included in the current request.
+   */
+  private static final String RA_INCLUDED_LIBRARY_PATHS = HtmlLibraryManager.class.getName() + ".included";
+
+  private final SlingHttpServletRequest request;
+
+  RequestIncludedLibraries(SlingHttpServletRequest request) {
+    this.request = request;
+  }
+
+  /**
+   * Gets set of library paths from request attribute. If not set, attribute is initialized with an empty set.
+   * @return Set of library paths attached to current request
+   */
+  @SuppressWarnings("unchecked")
+  private @NotNull Set<String> getLibaryPathsSetFromRequest() {
+    Set<String> libraryPaths = (Set<String>)request.getAttribute(RA_INCLUDED_LIBRARY_PATHS);
+    if (libraryPaths == null) {
+      libraryPaths = new HashSet<>();
+      request.setAttribute(RA_INCLUDED_LIBRARY_PATHS, libraryPaths);
+    }
+    return libraryPaths;
+  }
+
+  /**
+   * @param libraryPath Library path
+   * @return true if given library was already included in current request.
+   */
+  boolean isInlucded(@NotNull String libraryPath) {
+    return getLibaryPathsSetFromRequest().contains(libraryPath);
+  }
+
+  /**
+   * Store library path as included in current request.
+   * @param libraryPath Library path
+   */
+  void storeIncluded(@NotNull String libraryPath) {
+    getLibaryPathsSetFromRequest().add(libraryPath);
+  }
+
+}
diff --git a/src/test/java/io/wcm/wcm/ui/clientlibs/components/CSSIncludeTest.java b/src/test/java/io/wcm/wcm/ui/clientlibs/components/CSSIncludeTest.java
@@ -48,6 +48,15 @@ void testSingle() {
         underTest.getInclude());
   }
 
+  @Test
+  void testSingle_ContextPath() {
+    context.request().setContextPath("/mycontext");
+    context.request().setAttribute("categories", CATEGORY_SINGLE);
+    CSSInclude underTest = AdaptTo.notNull(context.request(), CSSInclude.class);
+    assertEquals("<link href=\"/mycontext/etc/clientlibs/app1/clientlib1.min.css\" rel=\"stylesheet\" type=\"text/css\">\n",
+        underTest.getInclude());
+  }
+
   @ParameterizedTest
   @ValueSource(strings = { "preload", "prefetch" })
   void testSingle_rel_valid(String validRelParameter) {
@@ -118,4 +127,25 @@ void testSingleNotAccessible() throws PersistenceException {
     assertNull(underTest.getInclude());
   }
 
+  @Test
+  void testMultiIgnoreDuplicates() {
+    context.request().setAttribute("categories", CATEGORIES_MULTIPLE);
+    CSSInclude underTest = AdaptTo.notNull(context.request(), CSSInclude.class);
+    assertEquals("<link href=\"/etc/clientlibs/app1/clientlib3.min.css\" rel=\"stylesheet\" type=\"text/css\">\n"
+        + "<link href=\"/etc.clientlibs/app1/clientlibs/clientlib4_proxy.min.css\" rel=\"stylesheet\" type=\"text/css\">\n"
+        + "<link href=\"/etc.clientlibs/app1/clientlibs/clientlib5_proxy.min.css\" rel=\"stylesheet\" type=\"text/css\">\n",
+        underTest.getInclude());
+
+    // include again - should not include anything
+    context.request().setAttribute("categories", CATEGORIES_MULTIPLE);
+    underTest = AdaptTo.notNull(context.request(), CSSInclude.class);
+    assertEquals("", underTest.getInclude());
+
+    // include something different - should include again
+    context.request().setAttribute("categories", CATEGORY_SINGLE);
+    underTest = AdaptTo.notNull(context.request(), CSSInclude.class);
+    assertEquals("<link href=\"/etc/clientlibs/app1/clientlib1.min.css\" rel=\"stylesheet\" type=\"text/css\">\n",
+        underTest.getInclude());
+  }
+
 }
diff --git a/src/test/java/io/wcm/wcm/ui/clientlibs/components/JSIncludeTest.java b/src/test/java/io/wcm/wcm/ui/clientlibs/components/JSIncludeTest.java
@@ -45,6 +45,14 @@ void testSingleNoAttributes() {
     assertEquals("<script src=\"/etc/clientlibs/app1/clientlib1.min.js\"></script>\n", underTest.getInclude());
   }
 
+  @Test
+  void testSingleNoAttributes_ContextPath() {
+    context.request().setContextPath("/mycontext");
+    context.request().setAttribute("categories", CATEGORY_SINGLE);
+    JSInclude underTest = AdaptTo.notNull(context.request(), JSInclude.class);
+    assertEquals("<script src=\"/mycontext/etc/clientlibs/app1/clientlib1.min.js\"></script>\n", underTest.getInclude());
+  }
+
   @Test
   void testSingleNoAttributesUnminified() {
     when(htmlLibraryManager.isMinifyEnabled()).thenReturn(false);
@@ -144,4 +152,25 @@ void testMultiAttributesWithCustom() {
         underTest.getInclude());
   }
 
+  @Test
+  void testMultiIgnoreDuplicates() {
+    context.request().setAttribute("categories", CATEGORIES_MULTIPLE);
+    JSInclude underTest = AdaptTo.notNull(context.request(), JSInclude.class);
+    assertEquals("<script src=\"/etc/clientlibs/app1/clientlib3.min.js\"></script>\n"
+        + "<script src=\"/etc.clientlibs/app1/clientlibs/clientlib4_proxy.min.js\"></script>\n"
+        + "<script src=\"/etc.clientlibs/app1/clientlibs/clientlib5_proxy.min.js\"></script>\n",
+        underTest.getInclude());
+
+    // include again - should not include anything
+    context.request().setAttribute("categories", CATEGORIES_MULTIPLE);
+    underTest = AdaptTo.notNull(context.request(), JSInclude.class);
+    assertEquals("", underTest.getInclude());
+
+    // include something different - should include again
+    context.request().setAttribute("categories", CATEGORY_SINGLE);
+    underTest = AdaptTo.notNull(context.request(), JSInclude.class);
+    assertEquals("<script src=\"/etc/clientlibs/app1/clientlib1.min.js\"></script>\n",
+        underTest.getInclude());
+  }
+
 }