Skip to content

Commit 43ddcc4

Browse files
fix(CopyContent): Moved Multitree validation before copyContentlet call + added DEFAULT variantId logic in PageResourceHelper (#34696)
## Summary Moved Multitree validation before copyContentlet call + added DEFAULT variantId logic in PageResourceHelper in case of null/empty values. ## Changes - Changed the validation order in PageResourceHelper.copyContentlet() method. Now we validate the existence of the Multitree before actually copying the Contentlet. - Added a default value for variantId in case the value from the request is null or empty. - Added integration tests. ## Motivation The issue related to this PR shows that we return a 404 when attempting to copy a contentlet and the variantId in the request is empty. To address this, we added logic to set a default value when the variantId is null or empty. Additionally, while investigating this issue, we uncovered another problem. The contentlet was being copied before doing the required validations for the Multitree. So when the Multitree call returned null, the response was still a 404, but the contentlet had already been copied, and the rollback was not being done. This means the bug was not limited to the empty variantId scenario. Any case where the Multitree was null could trigger the same unintended behavior. ## Related Issue Fixes: [BUG: PUT /api/v1/page/copyContent returns 404 but successfully copies content when variantId is empty string #34473](#34473)
1 parent bb1c594 commit 43ddcc4

2 files changed

Lines changed: 126 additions & 9 deletions

File tree

dotCMS/src/main/java/com/dotcms/rest/api/v1/page/PageResourceHelper.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -492,28 +492,27 @@ protected Contentlet copyContentlet(final CopyContentletForm copyContentletForm,
492492
final PageMode pageMode, final Language language)
493493
throws DotDataException, DotSecurityException {
494494

495-
final Tuple2<Contentlet, Contentlet> tuple2 = this.copyContent(copyContentletForm, user, pageMode, language.getId());
496-
497-
final Contentlet copiedContentlet = tuple2._1();
498-
final Contentlet originalContentlet = tuple2._2();
499495
final String htmlPage = copyContentletForm.getPageId();
500496
final String container = castToOriginalContainerId(copyContentletForm.getContainerId(), user, pageMode.respectAnonPerms);
501497
final String contentId = copyContentletForm.getContentId();
502498
final String instanceId = copyContentletForm.getRelationType();
503-
final String variant = copyContentletForm.getVariantId();
499+
final String variant = UtilMethods.isSet(copyContentletForm.getVariantId()) ? copyContentletForm.getVariantId() : VariantAPI.DEFAULT_VARIANT.name();
504500
final int treeOrder = copyContentletForm.getTreeOrder();
505501
final String personalization = copyContentletForm.getPersonalization();
506502
final Map<String, Object> styleProperties = copyContentletForm.getStyleProperties();
507503

508-
Logger.debug(this, ()-> "Deleting current contentlet multi tree: " + copyContentletForm);
509504
final MultiTree currentMultitree = getMultiTree(htmlPage, container, contentId, instanceId, personalization, variant);
510505

511506
if (null == currentMultitree) {
512-
513507
throw new DoesNotExistException(
514508
"Can not copied the contentlet in the page, because the record is not part of the page, multitree: " + copyContentletForm);
515509
}
516510

511+
final Tuple2<Contentlet, Contentlet> tuple2 = this.copyContent(copyContentletForm, user, pageMode, language.getId());
512+
final Contentlet copiedContentlet = tuple2._1();
513+
final Contentlet originalContentlet = tuple2._2();
514+
515+
Logger.debug(this, ()-> "Deleting current contentlet multi tree: " + copyContentletForm);
517516
APILocator.getMultiTreeAPI().deleteMultiTree(currentMultitree);
518517

519518
final MultiTree newMultitree = new MultiTree(htmlPage, container,

dotcms-integration/src/test/java/com/dotcms/rest/api/v1/page/PageResourceHelperTest.java

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.dotmarketing.beans.Host;
1010
import com.dotmarketing.beans.MultiTree;
1111
import com.dotmarketing.business.APILocator;
12+
import com.dotmarketing.exception.DoesNotExistException;
1213
import com.dotmarketing.exception.DotDataException;
1314
import com.dotmarketing.exception.DotSecurityException;
1415
import com.dotmarketing.portlets.containers.model.Container;
@@ -23,8 +24,6 @@
2324
import org.junit.BeforeClass;
2425
import org.junit.Test;
2526

26-
import java.util.Random;
27-
2827
import static org.junit.Assert.assertEquals;
2928
import static org.junit.Assert.assertNotEquals;
3029

@@ -107,6 +106,125 @@ public void copyContentlet() throws DotDataException, DotSecurityException {
107106
assertNotEquals(contentlet.getInode(), contentletCopy.getInode());
108107
}
109108

109+
/**
110+
* Method to test: {@link PageResourceHelper#copyContentlet(CopyContentletForm, User, PageMode, Language)}
111+
* when: Try to copy a Content from a Page where the Template that is not advanced and the Multi_tree has a
112+
* relation_type legacy value and the variantId is empty
113+
* should: copy the Contentlet anyway
114+
*
115+
* @throws DotDataException
116+
* @throws DotSecurityException
117+
*/
118+
@Test
119+
public void copyContentletWithEmptyVariantId() throws DotDataException, DotSecurityException {
120+
121+
final RandomString randomString = new RandomString();
122+
123+
final Field field_1 = new FieldDataGen()
124+
.name("field1")
125+
.velocityVarName("field1")
126+
.type(TextField.class)
127+
.next();
128+
129+
final Field field_2 = new FieldDataGen()
130+
.name("field2")
131+
.velocityVarName("field2")
132+
.type(TextField.class)
133+
.next();
134+
135+
final ContentType contentType = new ContentTypeDataGen()
136+
.field(field_1)
137+
.field(field_2)
138+
.nextPersisted();
139+
140+
final Contentlet contentlet = new ContentletDataGen(contentType.id())
141+
.setProperty("field1", randomString.nextString())
142+
.setProperty("field2", randomString.nextString())
143+
.nextPersisted();
144+
145+
final Container container = new ContainerDataGen().nextPersisted();
146+
final Host host = new SiteDataGen().nextPersisted();
147+
final Template template = new TemplateDataGen()
148+
.withContainer(container, ContainerUUID.UUID_LEGACY_VALUE)
149+
.nextPersisted();
150+
151+
final HTMLPageAsset page = new HTMLPageDataGen(host, template).nextPersisted();
152+
153+
new MultiTreeDataGen().setContentlet(contentlet)
154+
.setPage(page)
155+
.setContainer(container)
156+
.setPersonalization(MultiTree.DOT_PERSONALIZATION_DEFAULT)
157+
.setInstanceID(ContainerUUID.UUID_LEGACY_VALUE)
158+
.nextPersisted();
159+
160+
final CopyContentletForm copyContentletForm = new CopyContentletForm.Builder()
161+
.pageId(page.getIdentifier())
162+
.containerId(container.getIdentifier())
163+
.relationType("1")
164+
.contentId(contentlet.getIdentifier())
165+
.variantId("")
166+
.build();
167+
168+
final Language language = APILocator.getLanguageAPI().getLanguage(contentlet.getLanguageId());
169+
170+
final Contentlet contentletCopy = PageResourceHelper.getInstance().copyContentlet(copyContentletForm,
171+
APILocator.systemUser(), PageMode.PREVIEW_MODE, language);
172+
173+
assertEquals(contentlet.getStringProperty("field1"), contentletCopy.getStringProperty("field1"));
174+
assertEquals(contentlet.getStringProperty("field2"), contentletCopy.getStringProperty("field2"));
175+
176+
assertNotEquals(contentlet.getIdentifier(), contentletCopy.getIdentifier());
177+
assertNotEquals(contentlet.getInode(), contentletCopy.getInode());
178+
}
179+
180+
/**
181+
* Method to test: {@link PageResourceHelper#copyContentlet(CopyContentletForm, User, PageMode, Language)}
182+
* when: Try to copy a Content from a Page where the Multi_tree does not exist
183+
* should: throw DoesNotExistException
184+
*
185+
* @throws DotDataException
186+
* @throws DotSecurityException
187+
*/
188+
@Test(expected = DoesNotExistException.class)
189+
public void copyContentletThrowsDoesNotExistException() throws DotDataException, DotSecurityException {
190+
191+
final RandomString randomString = new RandomString();
192+
193+
final Field field_1 = new FieldDataGen()
194+
.name("field1")
195+
.velocityVarName("field1")
196+
.type(TextField.class)
197+
.next();
198+
199+
final ContentType contentType = new ContentTypeDataGen()
200+
.field(field_1)
201+
.nextPersisted();
202+
203+
final Contentlet contentlet = new ContentletDataGen(contentType.id())
204+
.setProperty("field1", randomString.nextString())
205+
.nextPersisted();
206+
207+
final Container container = new ContainerDataGen().nextPersisted();
208+
final Host host = new SiteDataGen().nextPersisted();
209+
final Template template = new TemplateDataGen()
210+
.withContainer(container, ContainerUUID.UUID_LEGACY_VALUE)
211+
.nextPersisted();
212+
213+
final HTMLPageAsset page = new HTMLPageDataGen(host, template).nextPersisted();
214+
215+
final CopyContentletForm copyContentletForm = new CopyContentletForm.Builder()
216+
.pageId(page.getIdentifier())
217+
.containerId(container.getIdentifier())
218+
.relationType("1")
219+
.contentId(contentlet.getIdentifier())
220+
.build();
221+
222+
final Language language = APILocator.getLanguageAPI().getLanguage(contentlet.getLanguageId());
223+
224+
PageResourceHelper.getInstance().copyContentlet(copyContentletForm,
225+
APILocator.systemUser(), PageMode.PREVIEW_MODE, language);
226+
}
227+
110228
/**
111229
* Method to test: {@link PageResourceHelper#copyContentlet(CopyContentletForm, User, PageMode, Language)}
112230
* when: Try to copy a Content from a Page where the Template is advanced and the Multi_tree has a

0 commit comments

Comments
 (0)