From 93fba5bacf1b208dd58f0976833947d54b07d953 Mon Sep 17 00:00:00 2001 From: RiccardoRuocco Date: Tue, 16 Jun 2026 11:55:16 +0200 Subject: [PATCH] fix-vanity-url-unpublish-canonical-page --- .../staticpublishing/AWSS3Publisher.java | 28 +++++++++ .../S3VanityAliasService.java | 58 +++++++++++++++---- ...> Task260507CreateS3VanityAliasTable.java} | 12 ++-- .../dotmarketing/util/TaskLocatorUtil.java | 4 +- dotCMS/src/main/resources/postgres.sql | 20 ++++++- .../S3VanityAliasServiceTest.java | 24 +++++++- ...VanityStaticPublishingIntegrationTest.java | 35 ++++++++++- 7 files changed, 159 insertions(+), 22 deletions(-) rename dotCMS/src/main/java/com/dotmarketing/startup/runonce/{Task260506CreateS3VanityAliasTable.java => Task260507CreateS3VanityAliasTable.java} (85%) diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/AWSS3Publisher.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/AWSS3Publisher.java index cfe89bf766ad..5c592cd84f8e 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/AWSS3Publisher.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/AWSS3Publisher.java @@ -388,6 +388,11 @@ public PublisherConfig process(final PublishStatus status) throws DotPublishingE endPointPublisher.pushBundleToEndpoint(bucketName, bucketRegion, bucketPrefix, filePath, file); } else { endPointPublisher.deleteFilesFromEndpoint(bucketName, bucketPrefix, filePath); + unpublishVanityAliasesForCanonicalFileIfEnabled(new S3VanityAliasContext( + new S3VanityAliasLookup(endpoint.getId(), host.getIdentifier(), + language.getId(), filePath), + bucketName, bucketRegion, bucketPrefix, host, language, file, + endPointPublisher)); } } catch(DotPublishingException e) { String error = updateStatusFailedToSend(currentStatusHistory, environment, endpoint, detail); @@ -646,6 +651,29 @@ private void publishVanityAliasesForCanonicalFilesIfEnabled(final AWSS3EndPointP } } + /** + * Removes vanity aliases materialized for a canonical file removed from S3. + * + * @param context canonical file context + * @throws DotPublishingException when alias cleanup fails + */ + private void unpublishVanityAliasesForCanonicalFileIfEnabled(final S3VanityAliasContext context) + throws DotPublishingException { + if (!isS3VanityAliasEnabled() || PublisherConfig.Operation.PUBLISH.equals(config.getOperation())) { + return; + } + + if (!context.endpointPublisher.acceptsFile(context.file)) { + return; + } + + try { + vanityAliasService.unpublishAliases(context); + } catch (final DotDataException e) { + throw new DotPublishingException(e.getMessage(), e); + } + } + /** * Handles one bundle element as a possible published Vanity URL. * diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasService.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasService.java index bd1b8fb10be6..bffd0f1623a7 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasService.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasService.java @@ -1,6 +1,5 @@ package com.dotcms.enterprise.publishing.staticpublishing; -import com.dotcms.business.WrapInTransaction; import com.dotcms.vanityurl.business.VanityUrlAPI; import com.dotcms.vanityurl.model.CachedVanityUrl; import com.dotcms.publishing.DotPublishingException; @@ -8,10 +7,13 @@ import com.dotmarketing.business.APILocator; import com.dotmarketing.business.DotStateException; import com.dotmarketing.business.PermissionAPI; +import com.dotmarketing.business.UserAPI; import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.portlets.contentlet.business.HostAPI; import com.dotmarketing.portlets.contentlet.model.Contentlet; import com.dotmarketing.portlets.htmlpageasset.business.HTMLPageAssetAPI; +import com.dotmarketing.portlets.languagesmanager.business.LanguageAPI; import com.dotmarketing.portlets.languagesmanager.model.Language; import com.dotmarketing.util.Constants; import com.dotmarketing.util.Logger; @@ -42,13 +44,17 @@ public class S3VanityAliasService { private final S3VanityAliasRepository repository; private final HTMLPageAssetAPI htmlPageAssetAPI; private final S3VanityTargetResolver targetResolver; + private final UserAPI userAPI; + private final HostAPI hostAPI; + private final LanguageAPI languageAPI; /** * Creates the service with system dependencies. */ public S3VanityAliasService() { this(APILocator.getVanityUrlAPI(), new S3VanityAliasSupport(), new S3VanityAliasRepository(), - APILocator.getHTMLPageAssetAPI(), new S3VanityTargetResolver()); + APILocator.getHTMLPageAssetAPI(), new S3VanityTargetResolver(), APILocator.getUserAPI(), + APILocator.getHostAPI(), APILocator.getLanguageAPI()); } /** @@ -61,7 +67,8 @@ public S3VanityAliasService() { public S3VanityAliasService(final VanityUrlAPI vanityUrlAPI, final S3VanityAliasSupport aliasSupport, final S3VanityAliasRepository repository) { this(vanityUrlAPI, aliasSupport, repository, APILocator.getHTMLPageAssetAPI(), - new S3VanityTargetResolver()); + new S3VanityTargetResolver(), APILocator.getUserAPI(), APILocator.getHostAPI(), + APILocator.getLanguageAPI()); } /** @@ -77,11 +84,37 @@ public S3VanityAliasService(final VanityUrlAPI vanityUrlAPI, final S3VanityAlias final S3VanityAliasRepository repository, final HTMLPageAssetAPI htmlPageAssetAPI, final S3VanityTargetResolver targetResolver) { + this(vanityUrlAPI, aliasSupport, repository, htmlPageAssetAPI, targetResolver, APILocator.getUserAPI(), + APILocator.getHostAPI(), APILocator.getLanguageAPI()); + } + + /** + * Creates the service with explicit dependencies for tests. + * + * @param vanityUrlAPI Vanity URL API + * @param aliasSupport alias support component + * @param repository alias mapping repository + * @param htmlPageAssetAPI HTML page rendering API + * @param targetResolver dotCMS target resolver + * @param userAPI user API + * @param hostAPI host API + * @param languageAPI language API + */ + S3VanityAliasService(final VanityUrlAPI vanityUrlAPI, final S3VanityAliasSupport aliasSupport, + final S3VanityAliasRepository repository, + final HTMLPageAssetAPI htmlPageAssetAPI, + final S3VanityTargetResolver targetResolver, + final UserAPI userAPI, + final HostAPI hostAPI, + final LanguageAPI languageAPI) { this.vanityUrlAPI = vanityUrlAPI; this.aliasSupport = aliasSupport; this.repository = repository; this.htmlPageAssetAPI = htmlPageAssetAPI; this.targetResolver = targetResolver; + this.userAPI = userAPI; + this.hostAPI = hostAPI; + this.languageAPI = languageAPI; } /** @@ -104,7 +137,7 @@ public void publishAliasForVanityUrl(final S3VanityAliasPublishContext context, return; } - final User systemUser = APILocator.getUserAPI().getSystemUser(); + final User systemUser = userAPI.getSystemUser(); final Optional canonicalPath = aliasSupport.normalizeCanonicalPath( aliasSupport.getForwardTo(vanityContentlet)); final Optional target; @@ -442,7 +475,7 @@ private void removeMaterializedAlias(final S3VanityAliasCleanupContext context, private Optional restoreLiveResource(final S3VanityAliasCleanupContext context, final S3VanityAlias alias) throws DotDataException, DotSecurityException, DotPublishingException { - final User systemUser = APILocator.getUserAPI().getSystemUser(); + final User systemUser = userAPI.getSystemUser(); final Optional restoreContext = buildRestoreContext(context, alias, systemUser); if (restoreContext.isEmpty()) { @@ -478,8 +511,8 @@ private Optional buildRestoreContext(final S3Vanity final S3VanityAlias alias, final User systemUser) throws DotDataException, DotSecurityException { - final Host host = APILocator.getHostAPI().find(alias.hostId, systemUser, false); - final Language language = APILocator.getLanguageAPI().getLanguage(alias.languageId); + final Host host = hostAPI.find(alias.hostId, systemUser, false); + final Language language = languageAPI.getLanguage(alias.languageId); if (host == null || language == null) { return Optional.empty(); } @@ -551,13 +584,18 @@ private void publishRestoredResource(final S3VanityAliasPublishContext context, */ public void unpublishAliases(final S3VanityAliasContext context) throws DotDataException { final List persistedAliases = repository.findByLookup(context.lookup); - final List deletedNow = new ArrayList<>(); + final S3VanityAliasCleanupContext cleanupContext = + new S3VanityAliasCleanupContext(context.lookup.endpointId, context.endpointPublisher); + final List removedNow = new ArrayList<>(); try { - deleteAliases(context, persistedAliases, deletedNow::add); + for (final S3VanityAlias alias : persistedAliases) { + removeMaterializedAlias(cleanupContext, alias); + removedNow.add(alias); + } repository.deleteByLookup(context.lookup); } catch (final Exception e) { - restoreAliases(context, deletedNow); + restoreAliases(context, removedNow); throw wrapAsDotDataException(e); } } diff --git a/dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task260506CreateS3VanityAliasTable.java b/dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task260507CreateS3VanityAliasTable.java similarity index 85% rename from dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task260506CreateS3VanityAliasTable.java rename to dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task260507CreateS3VanityAliasTable.java index a48db0283946..31606485b98e 100644 --- a/dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task260506CreateS3VanityAliasTable.java +++ b/dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task260507CreateS3VanityAliasTable.java @@ -10,7 +10,7 @@ /** * Creates the operational table that stores vanity aliases published on S3. */ -public class Task260506CreateS3VanityAliasTable extends AbstractJDBCStartupTask { +public class Task260507CreateS3VanityAliasTable extends AbstractJDBCStartupTask { private static final String TABLE_NAME = "static_s3_vanity_mapping"; @@ -46,14 +46,14 @@ public String getPostgresScript() { */ private String getScript() { return "CREATE TABLE IF NOT EXISTS static_s3_vanity_mapping (" // nosemgrep: gitlab.find_sec_bugs.CUSTOM_INJECTION-2 -- fully hardcoded DDL, no user input - + " endpoint_id varchar not null," - + " host_id varchar not null," + + " endpoint_id varchar(36) not null," + + " host_id varchar(36) not null," + " language_id bigint not null," + " canonical_path varchar not null," - + " canonical_path_hash varchar not null," + + " canonical_path_hash varchar(64) not null," + " vanity_path varchar not null," - + " vanity_path_hash varchar not null," - + " vanity_url_id varchar," + + " vanity_path_hash varchar(64) not null," + + " vanity_url_id varchar(36)," + " bucket_name varchar not null," + " bucket_region varchar," + " bucket_prefix varchar," diff --git a/dotCMS/src/main/java/com/dotmarketing/util/TaskLocatorUtil.java b/dotCMS/src/main/java/com/dotmarketing/util/TaskLocatorUtil.java index ea62a7d044b8..922715cf8f6d 100644 --- a/dotCMS/src/main/java/com/dotmarketing/util/TaskLocatorUtil.java +++ b/dotCMS/src/main/java/com/dotmarketing/util/TaskLocatorUtil.java @@ -266,8 +266,8 @@ import com.dotmarketing.startup.runonce.Task260403SetLz4CompressionOnTextColumns; import com.dotmarketing.startup.runonce.Task260403SetPermissionReferenceUnlogged; import com.dotmarketing.startup.runonce.Task260407AddBaseTypeColumnToIdentifier; -import com.dotmarketing.startup.runonce.Task260506CreateS3VanityAliasTable; import com.dotmarketing.startup.runonce.Task260505AddPluginsPortletToMenu; +import com.dotmarketing.startup.runonce.Task260507CreateS3VanityAliasTable; import com.dotmarketing.startup.runonce.Task260615AlterClusterIdLength; import com.google.common.collect.ImmutableList; @@ -609,8 +609,8 @@ public static List> getStartupRunOnceTaskClasses() { .add(Task260403SetLz4CompressionOnTextColumns.class) .add(Task260403SetPermissionReferenceUnlogged.class) .add(Task260407AddBaseTypeColumnToIdentifier.class) - .add(Task260506CreateS3VanityAliasTable.class) .add(Task260505AddPluginsPortletToMenu.class) + .add(Task260507CreateS3VanityAliasTable.class) .add(Task260615AlterClusterIdLength.class) .build(); diff --git a/dotCMS/src/main/resources/postgres.sql b/dotCMS/src/main/resources/postgres.sql index 1f5e5a70a016..bf7d6211714f 100644 --- a/dotCMS/src/main/resources/postgres.sql +++ b/dotCMS/src/main/resources/postgres.sql @@ -2281,6 +2281,25 @@ CREATE TABLE publishing_end_point ( auth_key text COMPRESSION lz4, sending bool); +CREATE TABLE static_s3_vanity_mapping ( + endpoint_id varchar(36) not null, + host_id varchar(36) not null, + language_id bigint not null, + canonical_path varchar not null, + canonical_path_hash varchar(64) not null, + vanity_path varchar not null, + vanity_path_hash varchar(64) not null, + vanity_url_id varchar(36), + bucket_name varchar not null, + bucket_region varchar, + bucket_prefix varchar, + mod_date timestamptz not null, + primary key (endpoint_id, host_id, language_id, canonical_path_hash, vanity_path_hash) +); + +CREATE INDEX idx_static_s3_vanity_mapping_vurl + ON static_s3_vanity_mapping (endpoint_id, vanity_url_id); + create table publishing_environment( id varchar(36) NOT NULL primary key, name varchar(255) NOT NULL unique, @@ -2599,4 +2618,3 @@ CREATE TABLE IF NOT EXISTS unique_fields unique_key_val VARCHAR PRIMARY KEY, supporting_values JSONB COMPRESSION lz4 ); - diff --git a/dotCMS/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasServiceTest.java b/dotCMS/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasServiceTest.java index 443a6abff332..3426899a7f9d 100644 --- a/dotCMS/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityAliasServiceTest.java @@ -4,8 +4,11 @@ import com.dotcms.vanityurl.business.VanityUrlAPI; import com.dotcms.vanityurl.model.CachedVanityUrl; import com.dotmarketing.beans.Host; +import com.dotmarketing.business.UserAPI; import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.portlets.contentlet.business.HostAPI; import com.dotmarketing.portlets.htmlpageasset.business.HTMLPageAssetAPI; +import com.dotmarketing.portlets.languagesmanager.business.LanguageAPI; import com.dotmarketing.portlets.languagesmanager.model.Language; import org.apache.http.HttpStatus; import org.junit.Assert; @@ -41,6 +44,11 @@ public class S3VanityAliasServiceTest { private final AWSS3EndPointPublisher endpointPublisher = mock(AWSS3EndPointPublisher.class); private final Host host = mock(Host.class); private final Language language = mock(Language.class); + private final HTMLPageAssetAPI htmlPageAssetAPI = mock(HTMLPageAssetAPI.class); + private final S3VanityTargetResolver targetResolver = mock(S3VanityTargetResolver.class); + private final UserAPI userAPI = mock(UserAPI.class); + private final HostAPI hostAPI = mock(HostAPI.class); + private final LanguageAPI languageAPI = mock(LanguageAPI.class); private final File canonicalFile = new File("canonical.html"); private final S3VanityAliasLookup lookup = new S3VanityAliasLookup(ENDPOINT_ID, HOST_ID, LANGUAGE_ID, CANONICAL_PATH); @@ -49,7 +57,7 @@ public class S3VanityAliasServiceTest { canonicalFile, endpointPublisher); private final S3VanityAliasService service = new S3VanityAliasService(vanityUrlAPI, new S3VanityAliasSupport(), repository, - mock(HTMLPageAssetAPI.class), mock(S3VanityTargetResolver.class)); + htmlPageAssetAPI, targetResolver, userAPI, hostAPI, languageAPI); @Test public void publishAliasesShouldReturnVoidAndRefreshOnlyPersistedAliases() throws Exception { @@ -122,6 +130,19 @@ public void unpublishAliasesShouldReturnVoidAndDeletePersistedAliases() throws E verify(endpointPublisher).deleteFilesFromEndpoint(BUCKET, PREFIX, "/promo"); verify(endpointPublisher).deleteFilesFromEndpoint(BUCKET, PREFIX, "/landing"); verify(repository).deleteByLookup(lookup); + verify(repository, never()).deleteAlias(any()); + } + + @Test + public void unpublishAliasesShouldReturnVoidAndSkipS3WhenNoAliasesArePersisted() throws Exception { + when(repository.findByLookup(lookup)).thenReturn(List.of()); + + final Void result = invokeVoid(() -> service.unpublishAliases(context)); + + Assert.assertNull(result); + verify(endpointPublisher, never()).deleteFilesFromEndpoint(any(), any(), any()); + verify(repository).deleteByLookup(lookup); + verify(repository, never()).deleteAlias(any()); } @Test @@ -137,6 +158,7 @@ public void unpublishAliasesShouldReturnDotDataExceptionAndKeepMappingWhenS3Dele Assert.assertNotNull(result); verify(repository, never()).deleteByLookup(lookup); + verify(repository, never()).deleteAlias(any()); } private Void invokeVoid(final VoidMethod method) throws Exception { diff --git a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityStaticPublishingIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityStaticPublishingIntegrationTest.java index 07f43c7812b7..0f6daa7a1a15 100644 --- a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityStaticPublishingIntegrationTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/staticpublishing/S3VanityStaticPublishingIntegrationTest.java @@ -32,7 +32,7 @@ import com.dotmarketing.portlets.htmlpageasset.model.HTMLPageAsset; import com.dotmarketing.portlets.languagesmanager.model.Language; import com.dotmarketing.portlets.templates.model.Template; -import com.dotmarketing.startup.runonce.Task260506CreateS3VanityAliasTable; +import com.dotmarketing.startup.runonce.Task260507CreateS3VanityAliasTable; import java.io.File; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -62,7 +62,7 @@ public class S3VanityStaticPublishingIntegrationTest { @BeforeClass public static void prepare() throws Exception { IntegrationTestInitService.getInstance().init(); - new Task260506CreateS3VanityAliasTable().executeUpgrade(); + new Task260507CreateS3VanityAliasTable().executeUpgrade(); } @Before @@ -224,6 +224,37 @@ public void unpublishAliasesShouldDeleteCloneAndMappingWhenCanonicalContentIsRem assertTrue(repository.findByLookup(lookup(source)).isEmpty()); } + /** + * Method to Test: {@link S3VanityAliasService#unpublishAliases(S3VanityAliasContext)} + * Given Scenario: A canonical page is removed and its materialized Vanity URL shadows a live dotCMS page + * ExpectedResult: The live page should be restored on the vanity key and the mapping should be removed. + */ + @Test + public void unpublishAliasesShouldRestoreShadowedLiveResourceWhenCanonicalContentIsRemoved() + throws Exception { + final Host host = new SiteDataGen().nextPersisted(); + final Language language = APILocator.getLanguageAPI().getDefaultLanguage(); + final PageFixture source = createLivePage(host, language, "source-canonical-shadow", + "canonical source content"); + final PageFixture shadowed = createLivePage(host, language, "canonical-shadowed-alias", + "shadowed canonical content"); + final Contentlet vanity = createLiveVanity(source, shadowed.path, source.path); + service.publishAliasForVanityUrl(context(source), vanity); + pushedPaths.clear(); + pushedContentByPath.clear(); + clearInvocations(endpointPublisher); + + service.unpublishAliases(aliasContext(source, temporaryStaticFile("unused"))); + + verify(endpointPublisher, never()).deleteFilesFromEndpoint(eq(BUCKET_NAME), eq(BUCKET_PREFIX), + eq(shadowed.path)); + verify(endpointPublisher).pushFileToEndpoint(eq(BUCKET_NAME), eq(BUCKET_REGION), + eq(BUCKET_PREFIX), eq(shadowed.path), any(File.class)); + assertEquals(List.of(shadowed.path), pushedPaths); + assertTrue(pushedContentByPath.get(shadowed.path).contains("shadowed canonical content")); + assertTrue(repository.findByLookup(lookup(source)).isEmpty()); + } + private void recordEndpointCalls() throws Exception { doAnswer(invocation -> { final String path = invocation.getArgument(3);