fix(Graphql): align vanityUrls resolution with Page API behavior dotCMS#32164 (dotCMS#32211)

valentinogiardino · web-flow · commit 6e003eb69755 · 2025-05-28T20:08:11.000Z
⚠️ **Breaking Change Notice** This change introduces a breaking behavior adjustment in the GraphQL `page` query to align its handling of **Vanity URLs** with the behavior of the **REST Page API**. ### Background On the REST `/api/v1/page/render/{uri}` endpoint: - If the requested URL is associated with a **temporary** or **permanent redirect**, the response contains: - An **empty** `page` object, and - A `vanityUrl` object describing the redirect (`forwardTo`, `action`, etc.). - If the Vanity URL is a **200 forward**, the response contains: - The **resolved page** (i.e., the target of the forward), and - The corresponding `vanityUrl` metadata. ### Previous Behavior in GraphQL (Inconsistent) When querying a page using a Vanity URL (e.g., `page(url: "/vanity-redirect")`), GraphQL would: - Return the `page` object of the **original URI**, even for redirects. - Also include the `vanityUrl` info. - It did **not** respect the redirect semantics (301/302), nor return an empty page as expected. ### New Behavior (Aligned with Page API) After this update: - For **temporary** or **permanent redirects**, GraphQL will return: - An **empty** `page` object, and - A `vanityUrl` object with proper redirect metadata. - For **200 forwards**, GraphQL will return: - The **resolved page content** (from the target of the forward), and - The associated `vanityUrl`. ### Impact Clients that rely on the GraphQL `page` query **will be affected** when querying URIs that are referenced by Vanity URLs. Specifically: - If the URI is associated with a **temporary (302)** or **permanent (301)** redirect: - The `page` object will now be **empty**, and - Only the `vanityUrl` metadata will be returned. - If the URI is associated with a **forward (200)**: - The returned `page` will represent the **resolved target page**, not the original URI. This change ensures GraphQL behaves consistently with the REST Page API, but may require consumers to update their logic to properly handle redirects or forwards based on `vanityUrl.action`. --- ### Proposed Changes - **PageDataFetcher** - Hook into the Vanity-URL API before rendering: 1. Resolve via `APILocator.getVanityUrlAPI().resolveVanityUrl(...)` 2. **Redirects**: return an “empty” `Contentlet` (so GraphQL can serialize `vanityUrl` but no page data) 3. **Forwards**: rewrite the fetch URI and continue to render the target page - Store the resolved `CachedVanityUrl` in the GraphQL context so downstream `VanityURLFetcher` can pick it up. - **VanityURLFetcher** - First check for a `cachedVanityUrl` in the GraphQL context and return it directly if present, falling back to the normal lookup otherwise. ### Checklist - [x] Tests - [x] Translations - [x] Security Implications Contemplated (add notes if applicable) ### Additional Info - Matches the REST behavior in `/api/v1/page/render/{uri}` - Verified manually in GraphQL Playground with three scenarios: ```graphql query VanityTest { forward: page(url: "/vanity-forward") { pageURI url vanityUrl { forwardTo uri action } } temporary: page(url: "/vanity-temporary-redirect") { pageURI url vanityUrl { forwardTo uri action } } permanent: page(url: "/vanity-permanent-redirect") { pageURI url vanityUrl { forwardTo uri action } } urlContentMapForward: page(url: "/blog/post/french-polynesia-everything-you-need-to-know") { pageURI url vanityUrl { forwardTo uri action } } } ![image](https://github.com/user-attachments/assets/efb073ec-946c-40e1-a7ed-1469cf777040)
diff --git a/dotCMS/src/main/java/com/dotcms/graphql/datafetcher/FieldDataFetcher.java b/dotCMS/src/main/java/com/dotcms/graphql/datafetcher/FieldDataFetcher.java
@@ -7,6 +7,7 @@
 import com.dotcms.contenttype.model.field.DataTypes;
 import com.dotcms.contenttype.model.field.Field;
 import com.dotcms.contenttype.model.field.TextField;
+import com.dotcms.contenttype.model.type.ContentType;
 import com.dotcms.graphql.DotGraphQLContext;
 import com.dotmarketing.portlets.contentlet.model.Contentlet;
 import com.dotmarketing.util.Logger;
@@ -27,7 +28,12 @@ public Object get(final DataFetchingEnvironment environment) throws Exception {
             Logger.debug(this, ()-> "Fetching field for contentlet: " + contentlet.getIdentifier() + " field: " + var);
             Object fieldValue = contentlet.get(var);
 
-            final Field field = contentlet.getContentType().fieldMap().get(var);
+            final ContentType contentType = contentlet.getContentType();
+            if (contentType == null) {
+                Logger.debug(this, ()-> "No ContentType on contentlet " + contentlet.getIdentifier() + ", returning raw value");
+                return fieldValue;
+            }
+            final Field field = contentType.fieldMap().get(var);
 
             if(!UtilMethods.isSet(fieldValue)) {
                 // null value - maybe a constant field
diff --git a/dotCMS/src/main/java/com/dotcms/graphql/datafetcher/page/PageDataFetcher.java b/dotCMS/src/main/java/com/dotcms/graphql/datafetcher/page/PageDataFetcher.java
@@ -1,11 +1,15 @@
 package com.dotcms.graphql.datafetcher.page;
 
+import com.dotcms.contenttype.model.type.ImmutablePageContentType;
 import com.dotcms.graphql.DotGraphQLContext;
 import com.dotcms.graphql.exception.PermissionDeniedGraphQLException;
 import com.dotcms.rest.api.v1.page.PageResource;
-import com.dotcms.variant.VariantAPI;
+import com.dotcms.vanityurl.business.VanityUrlAPI;
+import com.dotcms.vanityurl.model.CachedVanityUrl;
+import com.dotcms.vanityurl.model.VanityUrlResult;
 import com.dotmarketing.beans.Host;
 import com.dotmarketing.business.APILocator;
+import com.dotmarketing.business.web.WebAPILocator;
 import com.dotmarketing.exception.DotSecurityException;
 import com.dotmarketing.portlets.contentlet.model.Contentlet;
 import com.dotmarketing.portlets.contentlet.transform.DotContentletTransformer;
@@ -15,6 +19,7 @@
 import com.dotmarketing.portlets.htmlpageasset.business.render.PageContext;
 import com.dotmarketing.portlets.htmlpageasset.business.render.PageContextBuilder;
 import com.dotmarketing.portlets.htmlpageasset.model.HTMLPageAsset;
+import com.dotmarketing.portlets.languagesmanager.model.Language;
 import com.dotmarketing.portlets.rules.business.RulesEngine;
 import com.dotmarketing.portlets.rules.model.Rule.FireOn;
 import com.dotmarketing.util.DateUtil;
@@ -31,6 +36,7 @@
 import java.util.Date;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import java.util.Optional;
 
 /**
  * This DataFetcher returns a {@link HTMLPageAsset} given an URL. It also takes optional parameters
@@ -102,11 +108,42 @@ public Contentlet get(final DataFetchingEnvironment environment) throws Exceptio
                 }
             }
 
+            // Vanity URL resolution
+            String resolvedUri = url;
+            final Language language = UtilMethods.isSet(languageId) ?
+                    APILocator.getLanguageAPI().getLanguage(languageId) : APILocator.getLanguageAPI().getDefaultLanguage();
+            final Host host = WebAPILocator.getHostWebAPI().getHost(request);
+
+            final Optional<CachedVanityUrl> cachedVanityUrlOpt = APILocator.getVanityUrlAPI()
+                    .resolveVanityUrl(url, host, language);
+
+            if (cachedVanityUrlOpt.isPresent()) {
+                response.setHeader(VanityUrlAPI.VANITY_URL_RESPONSE_HEADER,
+                        cachedVanityUrlOpt.get().vanityUrlId);
+
+                // Store the CachedVanityUrl in the context
+                context.addParam("cachedVanityUrl", cachedVanityUrlOpt.get());
+
+                if (cachedVanityUrlOpt.get().isTemporaryRedirect() || 
+                    cachedVanityUrlOpt.get().isPermanentRedirect()) {
+                    // For redirects, return an empty page with vanity URL info
+                    final Contentlet emptyPage = new Contentlet();
+                    emptyPage.setLanguageId(language.getId());
+                    emptyPage.setHost(host.getIdentifier());
+                    return emptyPage;
+                } else {
+                    // For forwards, use the resolved URI
+                    final VanityUrlResult vanityUrlResult = cachedVanityUrlOpt.get()
+                            .handle(url);
+                    resolvedUri = vanityUrlResult.getRewrite();
+                }
+            }
+
             Logger.debug(this, ()-> "Fetching page for URL: " + url);
 
             final PageContext pageContext = PageContextBuilder.builder()
                     .setUser(user)
-                    .setPageUri(url)
+                    .setPageUri(resolvedUri)
                     .setPageMode(mode)
                     .setGraphQL(true)
                     .build();
@@ -130,6 +167,8 @@ public Contentlet get(final DataFetchingEnvironment environment) throws Exceptio
             final HTMLPageAsset pageAsset = pageUrl.getHTMLPage();
             context.addParam("page", pageAsset);
             pageAsset.getMap().put("URLMapContent", pageUrl.getUrlMapInfo());
+
+
             if(fireRules) {
                 Logger.info(this, "Rules will be fired");
                 request.setAttribute("fireRules", true);
diff --git a/dotCMS/src/main/java/com/dotcms/graphql/datafetcher/page/VanityURLFetcher.java b/dotCMS/src/main/java/com/dotcms/graphql/datafetcher/page/VanityURLFetcher.java
@@ -24,8 +24,14 @@ public class VanityURLFetcher implements DataFetcher<CachedVanityUrl> {
 
     @Override
     public CachedVanityUrl get(final DataFetchingEnvironment environment) throws Exception {
-
         final DotGraphQLContext context = environment.getContext();
+
+        // First check if we already have the cached vanity URL from PageDataFetcher
+        CachedVanityUrl cachedVanityUrl = (CachedVanityUrl) context.getParam("cachedVanityUrl");
+        if (cachedVanityUrl != null) {
+            return cachedVanityUrl;
+        }
+
         final User user = context.getUser();
         final Contentlet page = environment.getSource();
         final Host host = APILocator.getHostAPI().find(
@@ -39,6 +45,7 @@ public CachedVanityUrl get(final DataFetchingEnvironment environment) throws Exc
         if (!UtilMethods.isSet(uri)) {
             uri = page.getStringProperty("url");
         }
+
         if (UtilMethods.isSet(uri)) {
             final Optional<CachedVanityUrl> vanityUrlOpt = APILocator.getVanityUrlAPI().
                     resolveVanityUrl(uri, host, language);
@@ -49,5 +56,4 @@ public CachedVanityUrl get(final DataFetchingEnvironment environment) throws Exc
 
         return null;
     }
-
 }
diff --git a/dotcms-integration/src/test/java/com/dotcms/graphql/datafetcher/page/VanityURLFetcherTest.java b/dotcms-integration/src/test/java/com/dotcms/graphql/datafetcher/page/VanityURLFetcherTest.java
@@ -9,6 +9,7 @@
 import com.dotcms.graphql.DotGraphQLContext;
 import com.dotcms.util.FiltersUtil;
 import com.dotcms.util.IntegrationTestInitService;
+import com.dotcms.vanityurl.model.CachedVanityUrl;
 import com.dotmarketing.beans.Host;
 import com.dotmarketing.business.APILocator;
 import com.dotmarketing.business.UserAPI;
@@ -153,4 +154,38 @@ public void testGet_WithNonExitingVanityURL() throws Exception {
         assertNull(cachedVanityUrl);
     }
 
+    /**
+     * MethodToTest: {@link VanityURLFetcher#get(DataFetchingEnvironment)}
+     * Given Scenario: Context already includes a cached vanity URL
+     * Expected Result: Should return the cached vanity URL without resolving it again
+     */
+    @Test
+    public void testGet_WithCachedVanityUrlInContext() throws Exception {
+        final var fetcher = new VanityURLFetcher();
+
+        final var environment = Mockito.mock(DataFetchingEnvironment.class);
+        final var preCachedVanityUrl = new CachedVanityUrl(
+                "vanity-123",
+                "/pre-cached-url",
+                defaultLanguage.getId(),
+                defaultHost.getIdentifier(),
+                "/forwarded-destination",
+                301,
+                1
+        );
+
+        final var context = DotGraphQLContext.createServletContext()
+                .with(APILocator.systemUser())
+                .build();
+        context.addParam("cachedVanityUrl", preCachedVanityUrl);
+
+        Mockito.when(environment.getContext()).thenReturn(context);
+        Mockito.when(environment.getSource()).thenReturn(TestDataUtils.getPageContent(true, defaultLanguage.getId()));
+
+        final var cachedVanityUrl = fetcher.get(environment);
+        assertNotNull(cachedVanityUrl);
+        assertEquals("/pre-cached-url", cachedVanityUrl.url);
+        assertEquals("/forwarded-destination", cachedVanityUrl.forwardTo);
+        assertEquals(301, cachedVanityUrl.response);
+    }
 }
diff --git a/dotcms-postman/src/main/resources/postman/GraphQLTests.json b/dotcms-postman/src/main/resources/postman/GraphQLTests.json
