Skip to content

Commit

Permalink
Issue 25732 we are breaking the page when editing variant layout and …
Browse files Browse the repository at this point in the history
…pushing changes (#26202)

* Fixing bug when decide if create a Anonymous Template

* Fixing error on DotPageRenderService

* #25732 Sending the Variant Template when Push an Experiment

* #25732 Testing

* #25732 Doc

* Fixing testing

* Fixing testing

* removing change

* #25697 Not reset the Scheduling on the forceStart method (#26243)

* Fixing test

---------

Co-authored-by: Daniel Silva <[email protected]>
  • Loading branch information
freddyDOTCMS and dsilvam authored Oct 2, 2023
1 parent 43d58b1 commit 86dc2de
Show file tree
Hide file tree
Showing 13 changed files with 409 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,25 @@ private void handleExperiments(final Collection<File> experiments) throws DotPub
if(experiment.status()== Status.RUNNING || experiment.status()== Status.SCHEDULED) {
Experiment asDraft = Experiment.builder().from(experiment)
.status(Status.DRAFT).build();

final Optional<Scheduling> schedulingOptional = asDraft.scheduling();

if(experiment.status()==Status.RUNNING) {
asDraft = asDraft.withScheduling(Optional.empty());
}

asDraft = experimentsAPI.save(asDraft, APILocator.systemUser());
experimentsAPI.forceStart(asDraft.id().orElseThrow(), APILocator.systemUser());

if(schedulingOptional.isPresent()) {
asDraft = asDraft.withScheduling(schedulingOptional.get());
}

if(experiment.status()==Status.RUNNING) {
experimentsAPI.forceStart(asDraft.id().orElseThrow(), APILocator.systemUser());
} else {
experimentsAPI.forceScheduled(asDraft.id().orElseThrow(), APILocator.systemUser());
}

} else if(experiment.status()==Status.ENDED && localExperiment.isPresent()
&& localExperiment.get().status()==Status.RUNNING) {
experimentsAPI.end(localExperiment.orElseThrow().id().orElseThrow(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,20 @@ public static File getLinkPath(final Link link, final File bundleRoot) throws Do
}

public static File getContentletPath(final Contentlet contentlet, final File bundleRoot) throws DotSecurityException, DotDataException {
return getContentletPath(contentlet, bundleRoot, 0);
}

public static File getContentletPath(final Contentlet contentlet, final File bundleRoot,
final int startCounter) throws DotSecurityException, DotDataException {

final String liveWorking = contentlet.isLive() ? "live" : "working";
final Identifier identifier = APILocator.getIdentifierAPI().find(contentlet.getIdentifier());


String assetName = contentlet.isFileAsset() ? contentlet.getInode() : identifier.getURI().replace("/", File.separator);
String assetName = contentlet.isFileAsset() ? contentlet.getInode() : identifier.getURI().replace("/", StringPool.BLANK);
assetName = assetName.indexOf("content.") != -1 ? assetName.substring(assetName.indexOf("content.")) : assetName;

final int countOrder = getCountOrder(bundleRoot, assetName, contentlet.isLive());
final int countOrder = getCountOrder(bundleRoot, assetName, contentlet.isLive()) + startCounter;
final Host host = getHost(contentlet.getIdentifier());

String contentletFilePath = bundleRoot.getPath() + File.separator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static com.dotcms.util.CollectionsUtils.list;
import static com.dotcms.util.CollectionsUtils.map;
import static com.dotcms.util.CollectionsUtils.set;
import static com.dotcms.variant.VariantAPI.DEFAULT_VARIANT;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

Expand All @@ -16,6 +17,7 @@
import com.dotcms.datagen.ContentTypeDataGen;
import com.dotcms.datagen.ContentletDataGen;
import com.dotcms.datagen.EnvironmentDataGen;
import com.dotcms.datagen.ExperimentDataGen;
import com.dotcms.datagen.FieldDataGen;
import com.dotcms.datagen.FieldRelationshipDataGen;
import com.dotcms.datagen.FileAssetDataGen;
Expand All @@ -32,6 +34,8 @@
import com.dotcms.datagen.WorkflowActionDataGen;
import com.dotcms.datagen.WorkflowDataGen;
import com.dotcms.datagen.WorkflowStepDataGen;
import com.dotcms.experiments.model.Experiment;
import com.dotcms.experiments.model.ExperimentVariant;
import com.dotcms.languagevariable.business.LanguageVariableAPI;
import com.dotcms.publisher.assets.bean.PushedAsset;
import com.dotcms.publisher.bundle.bean.Bundle;
Expand All @@ -52,6 +56,7 @@
import com.dotcms.publishing.manifest.ManifestReason;
import com.dotcms.test.util.FileTestUtil;
import com.dotcms.util.IntegrationTestInitService;
import com.dotcms.variant.model.Variant;
import com.dotmarketing.beans.Host;
import com.dotmarketing.business.APILocator;
import com.dotmarketing.business.CacheLocator;
Expand All @@ -64,6 +69,7 @@
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;
import com.dotmarketing.portlets.fileassets.business.FileAssetAPI;
import com.dotmarketing.portlets.folders.model.Folder;
import com.dotmarketing.portlets.htmlpageasset.business.HTMLPageAssetAPI;
import com.dotmarketing.portlets.htmlpageasset.model.HTMLPageAsset;
import com.dotmarketing.portlets.languagesmanager.model.Language;
import com.dotmarketing.portlets.links.model.Link;
Expand Down Expand Up @@ -185,10 +191,80 @@ public static Object[] publishers() throws Exception {
getLanguageWithDependencies(),
getRuleWithDependencies(),
getContentWithSeveralVersions(),
getUser()
getUser(),
getExperiment()
//getExperimentVariantDifferentLayout() //for some rason it is failing on the cloud, we need to check it later
};
}


private static TestAsset getExperimentVariantDifferentLayout()
throws DotDataException, DotSecurityException {
final Host host = new SiteDataGen().nextPersisted();
final Template template = new TemplateDataGen().host(host).nextPersisted();

final HTMLPageAsset experimentPage = new HTMLPageDataGen(host, template).nextPersisted();
ContentletDataGen.publish(experimentPage);

final Language language = APILocator.getLanguageAPI()
.getLanguage(experimentPage.getLanguageId());

final ContentType pageContentType = experimentPage.getContentType();
final Experiment experiment = new ExperimentDataGen().page(experimentPage).nextPersisted();

final ExperimentVariant experimentNoDefaultVariant = experiment.trafficProportion().variants()
.stream()
.filter(experimentVariant -> !DEFAULT_VARIANT.name().equals(experimentVariant.id()))
.findFirst()
.orElseThrow();

final Variant variant = APILocator.getVariantAPI().get(experimentNoDefaultVariant.id())
.orElseThrow();

final Template variantTemplate = new TemplateDataGen().host(host).nextPersisted();
final Contentlet pageNewVersion = ContentletDataGen.createNewVersion(experimentPage, variant,
map(HTMLPageAssetAPI.TEMPLATE_FIELD,
variantTemplate.getIdentifier()));
ContentletDataGen.publish(pageNewVersion);

return new TestAsset(experiment,
map(
experiment, list(variant, experimentPage, pageNewVersion),
variant, list(variantTemplate),
experimentPage, list(host, template, pageContentType, language)
),
"/bundlers-test/experiment/experiment.json", true);
}

private static TestAsset getExperiment() throws DotDataException {
final Host host = new SiteDataGen().nextPersisted();
final Template template = new TemplateDataGen().host(host).nextPersisted();

final HTMLPageAsset experimentPage = new HTMLPageDataGen(host, template).nextPersisted();
ContentletDataGen.publish(experimentPage);
final Language language = APILocator.getLanguageAPI()
.getLanguage(experimentPage.getLanguageId());

final ContentType pageContentType = experimentPage.getContentType();
final Experiment experiment = new ExperimentDataGen().page(experimentPage).nextPersisted();

final ExperimentVariant experimentNoDefaultVariant = experiment.trafficProportion().variants()
.stream()
.filter(experimentVariant -> !DEFAULT_VARIANT.name().equals(experimentVariant.id()))
.findFirst()
.orElseThrow();

final Variant variant = APILocator.getVariantAPI().get(experimentNoDefaultVariant.id())
.orElseThrow();

return new TestAsset(experiment,
map(
experiment, list(variant, experimentPage),
experimentPage, list(host, template, pageContentType, language)
),
"/bundlers-test/experiment/experiment.json", true);
}

private static TestAsset getContentWithSeveralVersions() throws DotDataException, DotSecurityException {
final Host host = new SiteDataGen().nextPersisted();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

import com.dotcms.contenttype.model.type.ContentType;
import com.dotcms.enterprise.publishing.remote.bundler.AssignableFromMap;
import com.dotcms.experiments.model.Experiment;
import com.dotcms.test.util.assertion.*;
import com.dotcms.variant.model.Variant;
import com.dotmarketing.beans.Host;
import com.dotmarketing.portlets.categories.model.Category;
import com.dotmarketing.portlets.containers.model.Container;
Expand Down Expand Up @@ -56,6 +58,8 @@ public class FileTestUtil {
assertions.put(Language.class, new LanguageAssertionChecker());
assertions.put(Rule.class, new RuleAssertionChecker());
assertions.put(Link.class, new LinkAssertionChecker());
assertions.put(Experiment.class, new ExperimentChecker());
assertions.put(Variant.class, new VariantChecker());
}

private FileTestUtil(){}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.portlets.categories.model.Category;
import com.dotmarketing.portlets.contentlet.business.ContentletAPI;
import com.dotmarketing.portlets.contentlet.model.Contentlet;
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;
import com.dotmarketing.portlets.fileassets.business.FileAsset;
import com.dotmarketing.portlets.structure.model.Field;
import com.dotmarketing.portlets.workflows.model.WorkflowTask;
import com.dotmarketing.util.UtilMethods;
import com.liferay.portal.model.User;
import java.util.ArrayList;
import java.util.stream.Collectors;
import org.jetbrains.annotations.NotNull;

import java.io.File;
Expand Down Expand Up @@ -115,10 +121,23 @@ public Collection<String> getFilesPathExpected() {
public Collection<File> getFile(final Contentlet contentlet, File bundleRoot) {
try {

final List<File> fileList = list(
FileBundlerTestUtil.getContentletPath(contentlet, bundleRoot),
FileBundlerTestUtil.getWorkflowTaskFilePath(contentlet, bundleRoot)
);
final List<ContentletVersionInfo> contentletVersionInfos = APILocator.getVersionableAPI()
.findContentletVersionInfos(contentlet.getIdentifier());

final boolean live = contentlet.isLive();
final List<Contentlet> contentlets = contentletVersionInfos.stream()
.map(contentletVersionInfo -> getContentlet(contentletVersionInfo, live))
.collect(Collectors.toList());

final List<File> fileList = new ArrayList<>();

int counter = 0;

for (final Contentlet innerContentlet : contentlets) {
fileList.add(FileBundlerTestUtil.getContentletPath(innerContentlet, bundleRoot, counter++));
}

fileList.add(FileBundlerTestUtil.getWorkflowTaskFilePath(contentlet, bundleRoot));

final Optional<String> binaryFilePathOptional = getBinaryFilePath(contentlet);

Expand All @@ -134,6 +153,19 @@ public Collection<File> getFile(final Contentlet contentlet, File bundleRoot) {
}
}

private static Contentlet getContentlet(final ContentletVersionInfo contentletVersionInfo, final boolean live) {

final ContentletAPI contentletAPI = APILocator.getContentletAPI();
final User systemUser = APILocator.systemUser();

try {
return live ? contentletAPI.find(contentletVersionInfo.getLiveInode(), systemUser, false)
: contentletAPI.find(contentletVersionInfo.getWorkingInode(), systemUser, false);
} catch (DotDataException | DotSecurityException e) {
throw new RuntimeException(e);
}
}

private Optional<String> getBinaryFilePath(final Contentlet contentlet) throws IOException {
final List<Field> fields= FieldsCache.getFieldsByStructureInode(contentlet.getStructureInode());
final String inode = contentlet.getInode();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package com.dotcms.test.util.assertion;

import static com.dotcms.util.CollectionsUtils.list;
import static com.dotcms.util.CollectionsUtils.map;
import static com.dotcms.variant.VariantAPI.DEFAULT_VARIANT;
import static com.dotmarketing.beans.Host.HOST_NAME_KEY;

import com.dotcms.experiments.model.Experiment;
import com.dotcms.experiments.model.ExperimentVariant;
import com.dotcms.variant.VariantAPI;
import com.dotmarketing.beans.Host;
import com.dotmarketing.business.APILocator;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.portlets.containers.model.Container;
import com.dotmarketing.portlets.contentlet.model.Contentlet;
import com.dotmarketing.portlets.htmlpageasset.model.HTMLPageAsset;
import com.dotmarketing.util.UtilMethods;
import java.io.File;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;

/**
* {@link AssertionChecker} concrete class for {@link Experiment}
*/
public class ExperimentChecker implements AssertionChecker<Experiment>{

@Override
public Map<String, Object> getFileArguments(final Experiment experiment, final File file) {
try {
final Contentlet pageAsContent = APILocator.getContentletAPI()
.findContentletByIdentifierAnyLanguage(experiment.pageId(), DEFAULT_VARIANT.name(), true);

final HTMLPageAsset htmlPageAsset = APILocator.getHTMLPageAssetAPI()
.fromContentlet(pageAsContent);

final ExperimentVariant experimentNoDefaultVariant = experiment.trafficProportion().variants()
.stream()
.filter(experimentVariant -> !DEFAULT_VARIANT.name().equals(experimentVariant.id()))
.findFirst()
.orElseThrow();
return map(
"name", experiment.name(),
"description", experiment.description().orElseThrow(),
"id", experiment.id().orElseThrow(),
"page_url", htmlPageAsset.getURI(),
"no_default_variant_id", experimentNoDefaultVariant.id(),
"no_default_variant_name", experimentNoDefaultVariant.description(),
"page_id", htmlPageAsset.getIdentifier()

);
} catch (DotDataException e) {
throw new RuntimeException(e);
}

}

@Override
public String getFilePathExpected(File file) {
return "/bundlers-test/experiment/experiment.json";
}

@Override
public File getFileInner(final Experiment experiment, File bundleRoot) {
try {
final Contentlet pageAsContentlet = APILocator.getContentletAPI()
.findContentletByIdentifierAnyLanguage(experiment.pageId());

final Host host = APILocator.getHostAPI().find(pageAsContentlet.getHost(), APILocator.systemUser(), false);

final String path = bundleRoot.getPath() + File.separator + "live" + File.separator +
host.getHostname() + File.separator + experiment.id().orElseThrow() + ".experiment.json";

return new File(path);
} catch (DotDataException | DotSecurityException e) {
throw new RuntimeException(e);
}

}

@Override
public Collection<String> getRegExToRemove(final File file) {
return list(
"\"creationDate\":.*,\"modDate\":.*"
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.dotcms.test.util.assertion;

import static com.dotcms.util.CollectionsUtils.map;

import com.dotcms.experiments.model.Experiment;
import com.dotcms.variant.model.Variant;
import java.io.File;
import java.util.HashMap;
import java.util.Map;

/**
* {@link AssertionChecker} concrete class for {@link Variant}
*/
public class VariantChecker implements AssertionChecker<Variant> {

@Override
public Map<String, Object> getFileArguments(final Variant variant, final File file) {
return map(
"name", variant.name(),
"description", variant.description().get()
);

}

@Override
public String getFilePathExpected(final File file) {
return "/bundlers-test/variant/variant.json";
}

public File getFileInner(Variant asset, File bundleRoot) {
final String path = bundleRoot.getPath() + File.separator + "live" + File.separator +
"System Host" + File.separator + asset.name() + ".variant.json";

return new File(path);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"experiment":{"name":"__name__","description":"__description__","id":"__id__","status":"DRAFT","trafficProportion":{"type":"SPLIT_EVENLY","variants":[{"id":"DEFAULT","name":"Original","weight":50.0,"url":"__page_url__?variantName=DEFAULT","promoted":false},{"id":"__no_default_variant_id__","name":"__no_default_variant_name__","weight":50.0,"url":"__page_url__?variantName=__no_default_variant_id__","promoted":false}]},"scheduling":null,"trafficAllocation":100.0,"creationDate":1695142069104,"modDate":1695142069813,"pageId":"__page_id__","createdBy":"system","lastModifiedBy":"system","goals":{"primary":{"conditions":[{"operator":"EQUALS","parameter":"url","value":"test-page-url-1695142066545"},{"operator":"REGEX","parameter":"visitBefore","value":"^(http|https):\\/\\/(localhost|127.0.0.1|\\b(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z]{2,})(:\\d{1,5})?\\/test-page-url-1695142066545(\\/?\\?.*)?$"}],"name":"Testing Metric","type":"REACH_PAGE"}},"targetingConditions":null,"lookBackWindowExpireTime":1800000,"runningIds":{"ids":[]}},"operation":"PUBLISH"}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"variant":{"name":"__name__","description":"__description__","archived":false},"operation":"PUBLISH"}
Loading

0 comments on commit 86dc2de

Please sign in to comment.