From 9c931a75dcddcbc0aad1e908b8877155513d5f1d Mon Sep 17 00:00:00 2001 From: Will Ezell Date: Wed, 31 Jul 2024 14:01:15 -0400 Subject: [PATCH 1/5] fix(pp) fixes a case where pp fails if the pushed content has tags that live on a host that is not on reciever ref: #29397 --- .../remote/handler/ContentHandler.java | 25 +++++++- .../remote/handler/ContentHandlerTest.java | 62 +++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java index 5719dbde4a24..ef3fbb97cb92 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java @@ -114,10 +114,13 @@ import com.dotmarketing.util.PushPublishLogger.PushPublishHandler; import com.dotmarketing.util.UUIDUtil; import com.dotmarketing.util.UtilMethods; +import com.google.common.annotations.VisibleForTesting; import com.liferay.portal.model.User; import com.liferay.util.FileUtil; import com.thoughtworks.xstream.XStream; +import io.vavr.API; import io.vavr.Lazy; +import io.vavr.control.Try; import org.apache.commons.lang3.tuple.Pair; import java.io.File; @@ -364,6 +367,10 @@ private void handleContents(final Collection contents, final File folderOu } catch (final FileAssetValidationException e1){ Logger.error(ContentHandler.class, "Content id ["+content.getIdentifier()+"] could not be processed because of missing binary file. Error: "+e1.getMessage(),e1); } + catch (Exception e){ + Logger.error(this, "Error processing content in file " + workingOn, e); + throw new DotPublishingException("Error processing content in file " + workingOn + " : " + e.getMessage(), e); + } } workingOn = null; for (final File contentFile : contents) { @@ -1056,7 +1063,8 @@ private String getUniqueMatchErrorMsg(final List uniqueFields, final Stri * @param tagsFromSender - The list of {@link Tag} objects coming from the sender, * @throws DotDataException Tags could not be read or saved to the data source. */ - private void relateTagsToContent(Contentlet content, Map> tagsFromSender) throws DotDataException { + @VisibleForTesting + void relateTagsToContent(Contentlet content, Map> tagsFromSender) throws DotDataException { if(tagsFromSender!=null) { for (Map.Entry> fieldTags : tagsFromSender.entrySet()) { String fieldVarName = fieldTags.getKey(); @@ -1064,9 +1072,22 @@ private void relateTagsToContent(Contentlet content, Map> tags for (Tag remoteTag : fieldTags.getValue()) { Tag localTag = tagAPI.getTagByNameAndHost(remoteTag.getTagName(), remoteTag.getHostId()); + String localUserId = Try.of(()->APILocator.getUserAPI().loadUserById(remoteTag.getUserId()).getUserId()).getOrElse(APILocator.systemUser().getUserId()); + + Host tagSite = Try.of(()->APILocator.getHostAPI().find(remoteTag.getHostId(), APILocator.systemUser(), false)).getOrNull(); + Host contentSite = Try.of(()->APILocator.getHostAPI().find(content.getIdentifier(), APILocator.systemUser(), false)).getOrNull(); + + final String localSiteId = UtilMethods.isSet(()->tagSite.getTagStorage()) + ? tagSite.getTagStorage() + : UtilMethods.isSet(()->contentSite.getTagStorage()) + ? contentSite.getTagStorage() + : Host.SYSTEM_HOST; + + + // if there is NO local tag, save the one coming from remote, otherwise use local if (localTag == null || Strings.isNullOrEmpty(localTag.getTagId())) { - localTag = tagAPI.saveTag(remoteTag.getTagName(), remoteTag.getUserId(), remoteTag.getHostId()); + localTag = tagAPI.saveTag(remoteTag.getTagName(), localUserId, localSiteId); } TagInode localTagInode = tagAPI.getTagInode(localTag.getTagId(), content.getInode(), fieldVarName); diff --git a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java index 5f3d4dcd174d..c489a56d3322 100644 --- a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java @@ -1,20 +1,37 @@ package com.dotcms.enterprise.publishing.remote.handler; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; +import com.dotcms.contenttype.model.field.Field; +import com.dotcms.contenttype.model.field.TagField; +import com.dotcms.contenttype.model.type.ContentType; +import com.dotcms.datagen.ContentTypeDataGen; +import com.dotcms.datagen.ContentletDataGen; +import com.dotcms.datagen.TestDataUtils; import com.dotcms.publisher.pusher.wrapper.ContentWrapper; +import com.dotcms.publishing.PublisherConfig; import com.dotcms.test.util.FileTestUtil; import com.dotcms.util.IntegrationTestInitService; import com.dotcms.util.xstream.XStreamHandler; import com.dotcms.util.xstream.XStreamHandler.TrustedListMatcher; +import com.dotmarketing.beans.Host; +import com.dotmarketing.business.APILocator; import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.tag.model.Tag; +import com.dotmarketing.util.UUIDGenerator; import com.thoughtworks.xstream.XStream; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; +import java.util.List; +import java.util.Map; import org.junit.BeforeClass; import org.junit.Test; @@ -68,4 +85,49 @@ public void Test_TrustedListMatcher() { assertFalse(TrustedListMatcher.matches(disallowedClass2)); } + @Test + public void TEST_SAVING_TAGS_ON_NON_EXISTING_HOST() throws Exception{ + // Given + final String nonExistantHost = "non-existing-host" + UUIDGenerator.shorty(); + final String[] tags = {"tag1_" + UUIDGenerator.shorty(),"tag2_" + UUIDGenerator.shorty()}; + final String nonExistantUser = UUIDGenerator.shorty(); + + Contentlet contentlet = TestDataUtils.getDotAssetLikeContentlet(); + + List tagList = new ArrayList<>(); + + Arrays.stream(tags).forEach(tag -> { + Tag t = new Tag(); + t.setTagName(tag); + t.setHostId(nonExistantHost); + t.setModDate(new Date()); + t.setUserId(nonExistantUser); + tagList.add(t); + }); + + Field field = contentlet.getContentType().fields(TagField.class).get(0); + + Map> fieldTags = Map.of(field.variable(), tagList); + + // Should not throw an error + new ContentHandler(new PublisherConfig()).relateTagsToContent(contentlet,fieldTags); + + + assertEquals(APILocator.getTagAPI().getTagsByName(tags[0]).size(), 1); + + Tag savedTag = APILocator.getTagAPI().getTagsByName(tags[0]).get(0); + assertTrue(savedTag!=null); + assertEquals(savedTag.getTagName(), tags[0]); + assertEquals(savedTag.getHostId(), Host.SYSTEM_HOST); + + savedTag = APILocator.getTagAPI().getTagsByName(tags[1]).get(0); + assertTrue(savedTag!=null); + assertEquals(savedTag.getTagName(), tags[1]); + assertEquals(savedTag.getHostId(), Host.SYSTEM_HOST); + } + + + + + } From 2fb0769163929b6ed31fefa130f4d065909c0472 Mon Sep 17 00:00:00 2001 From: Will Ezell Date: Wed, 31 Jul 2024 14:14:24 -0400 Subject: [PATCH 2/5] fix(pp) fixes a case where pp fails if the pushed content has tags that live on a host that is not on reciever ref: #29397 --- .../publishing/remote/handler/ContentHandlerTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java index c489a56d3322..cebad4154e0c 100644 --- a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java @@ -32,6 +32,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Objects; import org.junit.BeforeClass; import org.junit.Test; @@ -107,7 +108,7 @@ public void TEST_SAVING_TAGS_ON_NON_EXISTING_HOST() throws Exception{ Field field = contentlet.getContentType().fields(TagField.class).get(0); - Map> fieldTags = Map.of(field.variable(), tagList); + Map> fieldTags = Map.of(Objects.requireNonNull(field.variable()), tagList); // Should not throw an error new ContentHandler(new PublisherConfig()).relateTagsToContent(contentlet,fieldTags); @@ -116,14 +117,14 @@ public void TEST_SAVING_TAGS_ON_NON_EXISTING_HOST() throws Exception{ assertEquals(APILocator.getTagAPI().getTagsByName(tags[0]).size(), 1); Tag savedTag = APILocator.getTagAPI().getTagsByName(tags[0]).get(0); - assertTrue(savedTag!=null); + assertNotNull(savedTag); assertEquals(savedTag.getTagName(), tags[0]); - assertEquals(savedTag.getHostId(), Host.SYSTEM_HOST); + assertEquals(Host.SYSTEM_HOST, savedTag.getHostId()); savedTag = APILocator.getTagAPI().getTagsByName(tags[1]).get(0); - assertTrue(savedTag!=null); + assertNotNull(savedTag); assertEquals(savedTag.getTagName(), tags[1]); - assertEquals(savedTag.getHostId(), Host.SYSTEM_HOST); + assertEquals(Host.SYSTEM_HOST, savedTag.getHostId()); } From 2761a641c68cac88b78c433a30d15bfb167ea137 Mon Sep 17 00:00:00 2001 From: Will Ezell Date: Thu, 1 Aug 2024 09:15:36 -0400 Subject: [PATCH 3/5] fix(pp) fixes a case where pp fails if the pushed content has tags that live on a host that is not on reciever - sonar feedback \n\nref: #29397 --- .../remote/handler/ContentHandler.java | 67 +++++++++---------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java index ef3fbb97cb92..87c71cc538aa 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java @@ -118,7 +118,6 @@ import com.liferay.portal.model.User; import com.liferay.util.FileUtil; import com.thoughtworks.xstream.XStream; -import io.vavr.API; import io.vavr.Lazy; import io.vavr.control.Try; import org.apache.commons.lang3.tuple.Pair; @@ -367,10 +366,6 @@ private void handleContents(final Collection contents, final File folderOu } catch (final FileAssetValidationException e1){ Logger.error(ContentHandler.class, "Content id ["+content.getIdentifier()+"] could not be processed because of missing binary file. Error: "+e1.getMessage(),e1); } - catch (Exception e){ - Logger.error(this, "Error processing content in file " + workingOn, e); - throw new DotPublishingException("Error processing content in file " + workingOn + " : " + e.getMessage(), e); - } } workingOn = null; for (final File contentFile : contents) { @@ -1056,46 +1051,48 @@ private String getUniqueMatchErrorMsg(final List uniqueFields, final Stri matchedContent.getInode(), fieldsInfo.toString()); } - /** - * Associates a list of tags coming from the bundle to the specified local content. - * - * @param content - The {@link Contentlet} that will have the updated tags from the bundle. - * @param tagsFromSender - The list of {@link Tag} objects coming from the sender, - * @throws DotDataException Tags could not be read or saved to the data source. - */ - @VisibleForTesting - void relateTagsToContent(Contentlet content, Map> tagsFromSender) throws DotDataException { - if(tagsFromSender!=null) { - for (Map.Entry> fieldTags : tagsFromSender.entrySet()) { - String fieldVarName = fieldTags.getKey(); + /** + * Associates a list of tags coming from the bundle to the specified local content. + * + * @param content - The {@link Contentlet} that will have the updated tags from the bundle. + * @param tagsFromSender - The list of {@link Tag} objects coming from the sender, + * @throws DotDataException Tags could not be read or saved to the data source. + */ + @VisibleForTesting + void relateTagsToContent(Contentlet content, Map> tagsFromSender) throws DotDataException { + if(tagsFromSender==null || tagsFromSender.isEmpty()) { + return; + } + + for (Map.Entry> fieldTags : tagsFromSender.entrySet()) { + String fieldVarName = fieldTags.getKey(); - for (Tag remoteTag : fieldTags.getValue()) { - Tag localTag = tagAPI.getTagByNameAndHost(remoteTag.getTagName(), remoteTag.getHostId()); + for (Tag remoteTag : fieldTags.getValue()) { + Tag localTag = tagAPI.getTagByNameAndHost(remoteTag.getTagName(), remoteTag.getHostId()); - String localUserId = Try.of(()->APILocator.getUserAPI().loadUserById(remoteTag.getUserId()).getUserId()).getOrElse(APILocator.systemUser().getUserId()); + String localUserId = Try.of(()->APILocator.getUserAPI().loadUserById(remoteTag.getUserId()).getUserId()).getOrElse(APILocator.systemUser().getUserId()); - Host tagSite = Try.of(()->APILocator.getHostAPI().find(remoteTag.getHostId(), APILocator.systemUser(), false)).getOrNull(); - Host contentSite = Try.of(()->APILocator.getHostAPI().find(content.getIdentifier(), APILocator.systemUser(), false)).getOrNull(); + Host tagSite = Try.of(()->APILocator.getHostAPI().find(remoteTag.getHostId(), APILocator.systemUser(), false)).getOrNull(); + Host contentSite = Try.of(()->APILocator.getHostAPI().find(content.getIdentifier(), APILocator.systemUser(), false)).getOrNull(); - final String localSiteId = UtilMethods.isSet(()->tagSite.getTagStorage()) - ? tagSite.getTagStorage() - : UtilMethods.isSet(()->contentSite.getTagStorage()) + final String localSiteId = UtilMethods.isSet(()->tagSite.getTagStorage()) + ? tagSite.getTagStorage() + : UtilMethods.isSet(()->contentSite.getTagStorage()) ? contentSite.getTagStorage() - : Host.SYSTEM_HOST; + : Host.SYSTEM_HOST; - // if there is NO local tag, save the one coming from remote, otherwise use local - if (localTag == null || Strings.isNullOrEmpty(localTag.getTagId())) { - localTag = tagAPI.saveTag(remoteTag.getTagName(), localUserId, localSiteId); - } + // if there is NO local tag, save the one coming from remote, otherwise use local + if (localTag == null || Strings.isNullOrEmpty(localTag.getTagId())) { + localTag = tagAPI.saveTag(remoteTag.getTagName(), localUserId, localSiteId); + } - TagInode localTagInode = tagAPI.getTagInode(localTag.getTagId(), content.getInode(), fieldVarName); + TagInode localTagInode = tagAPI.getTagInode(localTag.getTagId(), content.getInode(), fieldVarName); - // avoid relating tags twice - if(localTagInode==null || !Strings.isNullOrEmpty(localTagInode.getTagId())) { - tagAPI.addContentletTagInode(localTag, content.getInode(), fieldVarName); - } + // avoid relating tags twice + if(localTagInode==null || !Strings.isNullOrEmpty(localTagInode.getTagId())) { + tagAPI.addContentletTagInode(localTag, content.getInode(), fieldVarName); } } } From 7da2f2d1042ab689ec72b5c597978e79ed73a84d Mon Sep 17 00:00:00 2001 From: Will Ezell Date: Thu, 1 Aug 2024 10:01:06 -0400 Subject: [PATCH 4/5] fix(pp) fixes a case where pp fails if the pushed content has tags that live on a host that is not on reciever - sonar feedback \n\nref: #29397 --- .../enterprise/publishing/remote/handler/ContentHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java index 87c71cc538aa..3a4ffa980339 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java @@ -1091,7 +1091,7 @@ void relateTagsToContent(Contentlet content, Map> tagsFromSend TagInode localTagInode = tagAPI.getTagInode(localTag.getTagId(), content.getInode(), fieldVarName); // avoid relating tags twice - if(localTagInode==null || !Strings.isNullOrEmpty(localTagInode.getTagId())) { + if(UtilMethods.isEmpty(()->localTagInode.getTagId())) { tagAPI.addContentletTagInode(localTag, content.getInode(), fieldVarName); } } From 774bbe2c4a138cf4bb2b419455e015e0b00f5d84 Mon Sep 17 00:00:00 2001 From: Will Ezell Date: Tue, 20 Aug 2024 17:17:55 -0400 Subject: [PATCH 5/5] fix(pp) adding notes to ContentHandlerTest ref: #29397 --- .../publishing/remote/handler/ContentHandlerTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java index cebad4154e0c..d423f1356872 100644 --- a/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java @@ -85,7 +85,11 @@ public void Test_TrustedListMatcher() { assertFalse(TrustedListMatcher.matches(disallowedClass1)); assertFalse(TrustedListMatcher.matches(disallowedClass2)); } - + /** + * Method to test: {@link ContentHandler#relateTagsToContent(Contentlet content, Map> tags)} + * When: Content is pushed which has tags that live on a host not in the target system. + * Should: The tags should save to the system host + */ @Test public void TEST_SAVING_TAGS_ON_NON_EXISTING_HOST() throws Exception{ // Given