fix(pp) fixes a case where pp fails if the pushed content has tags th…

dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandler.java

@@ -114,10 +114,12 @@
 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.Lazy;
+import io.vavr.control.Try;
 import org.apache.commons.lang3.tuple.Pair;
 
 import java.io.File;
@@ -1049,32 +1051,48 @@ private String getUniqueMatchErrorMsg(final List<Field> 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.
-     */
-	private void relateTagsToContent(Contentlet content, Map<String, List<Tag>> tagsFromSender) throws DotDataException {
-		if(tagsFromSender!=null) {
-			for (Map.Entry<String, List<Tag>> 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<String, List<Tag>> tagsFromSender) throws DotDataException {
+		if(tagsFromSender==null || tagsFromSender.isEmpty()) {
+			return;
+		}
 
-				for (Tag remoteTag : fieldTags.getValue()) {
-					Tag localTag = tagAPI.getTagByNameAndHost(remoteTag.getTagName(), remoteTag.getHostId());
+		for (Map.Entry<String, List<Tag>> fieldTags : tagsFromSender.entrySet()) {
+			String fieldVarName = fieldTags.getKey();
 
-					// 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());
-					}
+			for (Tag remoteTag : fieldTags.getValue()) {
+				Tag localTag = tagAPI.getTagByNameAndHost(remoteTag.getTagName(), remoteTag.getHostId());
 
-					TagInode localTagInode = tagAPI.getTagInode(localTag.getTagId(), content.getInode(), fieldVarName);
+				String localUserId = Try.of(()->APILocator.getUserAPI().loadUserById(remoteTag.getUserId()).getUserId()).getOrElse(APILocator.systemUser().getUserId());
 
-					// avoid relating tags twice
-					if(localTagInode==null || !Strings.isNullOrEmpty(localTagInode.getTagId())) {
-						tagAPI.addContentletTagInode(localTag, content.getInode(), fieldVarName);
-					}
+				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(), localUserId, localSiteId);
+				}
+
+				TagInode localTagInode = tagAPI.getTagInode(localTag.getTagId(), content.getInode(), fieldVarName);
+
+				// avoid relating tags twice
+				if(UtilMethods.isEmpty(()->localTagInode.getTagId())) {
+					tagAPI.addContentletTagInode(localTag, content.getInode(), fieldVarName);
 				}
 			}
 		}

dotcms-integration/src/test/java/com/dotcms/enterprise/publishing/remote/handler/ContentHandlerTest.java

@@ -1,20 +1,38 @@
 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 java.util.Objects;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -67,5 +85,54 @@ public void Test_TrustedListMatcher() {
         assertFalse(TrustedListMatcher.matches(disallowedClass1));
         assertFalse(TrustedListMatcher.matches(disallowedClass2));
     }
+    /**
+     * Method to test: {@link ContentHandler#relateTagsToContent(Contentlet content, Map<String, List<Tag>> 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
+        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<Tag> 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<String,List<Tag>> fieldTags = Map.of(Objects.requireNonNull(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);
+        assertNotNull(savedTag);
+        assertEquals(savedTag.getTagName(), tags[0]);
+        assertEquals(Host.SYSTEM_HOST, savedTag.getHostId());
+
+        savedTag = APILocator.getTagAPI().getTagsByName(tags[1]).get(0);
+        assertNotNull(savedTag);
+        assertEquals(savedTag.getTagName(), tags[1]);
+        assertEquals(Host.SYSTEM_HOST, savedTag.getHostId());
+    }
+
+
+
+
 
 }