Fix external redirect link not shown to author

bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/RedirectItemImpl.java

@@ -15,8 +15,8 @@
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
 package com.adobe.cq.wcm.core.components.internal.models.v1;
 
+import com.adobe.cq.wcm.core.components.internal.resource.CoreResourceWrapper;
 import org.apache.sling.api.SlingHttpServletRequest;
-import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -27,27 +27,67 @@
 import com.day.cq.wcm.api.Page;
 import com.day.cq.wcm.api.PageManager;
 
+import java.util.Collections;
+import java.util.Optional;
+
+import static com.adobe.cq.wcm.core.components.internal.models.v2.PageImpl.PN_REDIRECT_TARGET;
+
+/**
+ * Navigation Item used for page redirects.
+ */
 public class RedirectItemImpl implements NavigationItem {
 
-    private final String redirectTarget;
+    /**
+     * The target page that this Redirect Item redirects to (null if page doesn't exist, or redirect does not point to a page).
+     */
     private final Page page;
-    protected final Link link;
 
-    public RedirectItemImpl(@NotNull String redirectTarget, @NotNull SlingHttpServletRequest request, @NotNull LinkManager linkManager) {
-        this.redirectTarget = redirectTarget;
-        this.page = getRedirectPage(request);
-        this.link = linkManager.get(this.page).build();
+    /**
+     * Link that this Redirect Item redirects to.
+     */
+    protected final Link<?> link;
+
+    /**
+     * Construct a Redirect Item.
+     *
+     * @param redirectTarget The path or URL to redirect to.
+     * @param request The current request.
+     * @param linkManager The Link Manager.
+     */
+    public RedirectItemImpl(@NotNull final String redirectTarget,
+                            @NotNull final SlingHttpServletRequest request,
+                            @NotNull final LinkManager linkManager) {
+        Optional<Page> redirectPage = getRedirectPage(request.getResourceResolver(), redirectTarget);
+        this.link = redirectPage
+            .map(linkManager::get)
+            .orElseGet(() -> {
+                // this wrapper exists to handle the possibility that `redirectTarget` didn't come from `PN_REDIRECT_TARGET`
+                CoreResourceWrapper resourceWrapper = new CoreResourceWrapper(
+                    request.getResource(),
+                    request.getResource().getResourceType(),
+                    Collections.emptyList(),
+                    Collections.singletonMap(PN_REDIRECT_TARGET, redirectTarget));
+                return linkManager.get(resourceWrapper).withLinkUrlPropertyName(PN_REDIRECT_TARGET);
+            })
+            .build();
+
+        this.page = Optional.ofNullable(this.link.getReference())
+            .filter(ref -> ref instanceof Page)
+            .map(ref -> (Page) ref)
+            .orElse(redirectPage.orElse(null));
     }
 
-    private Page getRedirectPage(@NotNull SlingHttpServletRequest request) {
-        Page page = null;
-        ResourceResolver resourceResolver = request.getResourceResolver();
-        Resource targetResource = resourceResolver.getResource(redirectTarget);
-        PageManager pageManager = resourceResolver.adaptTo(PageManager.class);
-        if (pageManager != null && targetResource != null) {
-            page = pageManager.getContainingPage(targetResource);
-        }
-        return page;
+    /**
+     * Get the redirect target page.
+     *
+     * @param resourceResolver A ResourceResolver.
+     * @param redirectTarget The target path.
+     * @return The target page is redirectTarget references a page, otherwise empty.
+     */
+    private Optional<Page> getRedirectPage(@NotNull final ResourceResolver resourceResolver, @NotNull final String redirectTarget) {
+        return Optional.ofNullable(resourceResolver.getResource(redirectTarget))
+            .flatMap(targetResource -> Optional.ofNullable(resourceResolver.adaptTo(PageManager.class))
+                .map(pm -> pm.getContainingPage(targetResource)));
     }
 
     @Override
@@ -59,6 +99,7 @@ public Page getPage() {
 
     @Override
     @Nullable
+    @Deprecated
     public String getURL() {
         return link.getURL();
     }

bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v2/PageImplTest.java

@@ -71,6 +71,7 @@ public class PageImplTest extends com.adobe.cq.wcm.core.components.internal.mode
 
     private static final String TEST_BASE = "/page/v2";
     protected static final String REDIRECT_PAGE = CONTENT_ROOT + "/redirect-page";
+    protected static final String REDIRECT_PAGE_EXTERNAL = CONTENT_ROOT + "/redirect-page-external";
     private static final String PN_CLIENT_LIBS = "clientlibs";
     private static final String SLING_CONFIGS_ROOT = "/conf/page/sling:configs";
 

bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v3/PageImplTest.java

@@ -28,6 +28,7 @@
 import static com.adobe.cq.wcm.core.components.Utils.skipDataLayerInclude;
 import static com.adobe.cq.wcm.core.components.internal.link.LinkTestUtils.assertValidLink;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -58,6 +59,17 @@
         assertValidLink(redirectTarget.getLink(), "/content/page/templated-page.html", context.request());
     }
 
+    @Test
+    @SuppressWarnings("deprecation")
+    protected void testRedirectTarget_external() {
+        Page page = getPageUnderTest(REDIRECT_PAGE_EXTERNAL);
+        NavigationItem redirectTarget = page.getRedirectTarget();
+        assertNotNull(redirectTarget);
+        assertNull(redirectTarget.getPage());
+        assertEquals("https://www.adobe.com/", redirectTarget.getURL());
+        assertValidLink(redirectTarget.getLink(), "https://www.adobe.com/", context.request());
+    }
+
     @Test
     protected void testIsClientlibsAsync_undefined() {
         Page page = getPageUnderTest(REDIRECT_PAGE);

bundles/core/src/test/resources/page/v3/test-content.json

@@ -64,5 +64,22 @@
       "cq:lastModified"   : "Wed Jan 20 2016 10:33:36 GMT+0100",
       "cq:redirectTarget" : "/content/page/templated-page"
     }
+  },
+  "redirect-page-external": {
+    "jcr:primaryType": "cq:Page",
+    "jcr:createdBy"  : "admin",
+    "jcr:created"    : "Wed Jan 20 2016 10:29:36 GMT+0100",
+    "jcr:content"    : {
+      "jcr:primaryType"   : "cq:PageContent",
+      "jcr:createdBy"     : "admin",
+      "jcr:title"         : "Redirect Page - External",
+      "jcr:description"   : "Description",
+      "cq:template"       : "/conf/coretest/settings/wcm/templates/product-page",
+      "sling:resourceType": "core/wcm/components/page/v3/page",
+      "jcr:created"       : "Wed Jan 20 2016 10:29:36 GMT+0100",
+      "jcr:language"      : "en_GB",
+      "cq:lastModified"   : "Wed Jan 20 2016 10:33:36 GMT+0100",
+      "cq:redirectTarget" : "https://www.adobe.com/"
+    }
   }
 }

content/src/content/jcr_root/apps/core/wcm/components/page/v3/page/redirect.html

@@ -15,6 +15,6 @@
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/-->
 <template data-sly-template.redirect="${ @ redirectTarget}">
     <p class="cmp-page__redirect">${'This page redirects to' @ i18n}
-        <a data-sly-attribute="${redirectTarget.link.htmlAttributes}" data-sly-unwrap="${!redirectTarget.link.valid}">${redirectTarget.page.title || redirectTarget.linkURL}</a>
+        <a data-sly-attribute="${redirectTarget.link.htmlAttributes}" data-sly-unwrap="${!redirectTarget.link.valid}">${redirectTarget.page.title || redirectTarget.link.URL}</a>
     </p>
 </template>