Skip to content

Commit

Permalink
fix(variantWebApi) dotAsset localization issue for non default language
Browse files Browse the repository at this point in the history 
#29851 (#30587)

### Proposed Changes
* Updated the `isFileFallback` method to account for content in
non-default languages.
- Added a conditional check for `DOTASSET` types and ensured fallback
behavior respects language settings via `LanguageAPI`.
* Updated the `inodePath` method in `ShortyServlet` to implement
language fallback logic for contentlets with `ImageField` or `FileField`
when the associated content version is missing in a specified language.

### Checklist
- [x] Tests
- [x] Translations
- [x] Security Implications Contemplated

### Additional Info
**Context**: This fix resolves an issue where content with image fields
in a secondary language did not display the associated image as
expected. The modification ensures that dotAsset images can fall back to
the default language if they are not available in the specified
language, preventing unexpected behaviors when creating or editing
multilingual pages.

### Screenshots
#### Current behavior (issue)


https://github.com/user-attachments/assets/81c0ae60-a5d5-4046-a77b-87dcd1195cf3

#### Behavior after the fix


https://github.com/user-attachments/assets/f9f44e12-9e1c-484c-b496-32efbd3c9c01
  • Loading branch information
@valentinogiardino
valentinogiardino authored Nov 8, 2024
1 parent 1f1e05d commit 824106f
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 42 deletions.
2 changes: 1 addition & 1 deletion dotCMS/src/main/java/com/dotcms/variant/business/web/VariantWebAPIImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ private static boolean isWidgetFallback(ContentType type) {
}

private static boolean isFileFallback(ContentType type) {
return type.baseType() == BaseContentType.FILEASSET
return (type.baseType() == BaseContentType.FILEASSET || type.baseType() == BaseContentType.DOTASSET)
&& APILocator.getLanguageAPI().canDefaultFileToDefaultLanguage();
}

Expand Down
87 changes: 69 additions & 18 deletions dotCMS/src/main/java/com/dotmarketing/servlets/ShortyServlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,20 @@ private boolean isValidRequest(final HttpServletRequest request,
return true;
}



/**
* Resolves and builds the appropriate file path for a contentlet's field.
* This method handles both regular fields and special cases for image/file fields,
* including language fallback logic when necessary.
*
* @param contentlet The contentlet whose field path needs to be resolved
* @param tryField The name of the field to try to resolve
* @param live Whether to use the live version (true) or working version (false)
* @return A string representing the path to the field's content
* @throws DotStateException If there's an issue with the contentlet's state
* @throws DotDataException If there's an error accessing the data
* @throws DotSecurityException If there's a security violation
*/
protected final String inodePath(final Contentlet contentlet,
final String tryField,
final boolean live)
Expand All @@ -508,35 +521,73 @@ protected final String inodePath(final Contentlet contentlet,
final Optional<Field> fieldOpt = resolveField(contentlet, tryField);

if (fieldOpt.isEmpty()) {
return "/" + contentlet.getInode() + "/" + FILE_ASSET_DEFAULT;
return buildFieldPath(contentlet, FILE_ASSET_DEFAULT);
}

final Field field = fieldOpt.get();
if (field instanceof ImageField || field instanceof FileField) {

final String relatedImageId = contentlet.getStringProperty(field.variable());
final Optional<ContentletVersionInfo> contentletVersionInfo =
Optional<ContentletVersionInfo> contentletVersionInfo =
this.versionableAPI.getContentletVersionInfo(relatedImageId, contentlet.getLanguageId());

if (contentletVersionInfo.isPresent()) {
final String inode = live ? contentletVersionInfo.get().getLiveInode()
: contentletVersionInfo.get().getWorkingInode();

final Contentlet imageContentlet = APILocator.getContentletAPI()
.find(inode, APILocator.systemUser(), false);

validateContentlet(imageContentlet, live, inode);

final String fieldVar = imageContentlet.isDotAsset() ?
DotAssetContentType.ASSET_FIELD_VAR : FILE_ASSET_DEFAULT;
if (contentletVersionInfo.isEmpty() && shouldFallbackToDefaultLanguage(contentlet)) {
// Try finding the contentlet version with the default language ID
Logger.info(this, "No contentlet version found for identifier " + relatedImageId + " in language " + contentlet.getLanguageId() + ", trying default language.");
contentletVersionInfo = this.versionableAPI.getContentletVersionInfo(relatedImageId,APILocator.getLanguageAPI().getDefaultLanguage().getId());
}

return new StringBuilder(StringPool.FORWARD_SLASH).append(inode)
.append(StringPool.FORWARD_SLASH).append(fieldVar).toString();
if (contentletVersionInfo.isPresent()) {
Logger.debug(this, "Contentlet version found for identifier: " + relatedImageId);
final Contentlet imageContentlet = getImageContentlet(contentletVersionInfo.get(), live);
validateContentlet(imageContentlet, live, imageContentlet.getInode());
final String fieldVar = imageContentlet.isDotAsset() ? DotAssetContentType.ASSET_FIELD_VAR : FILE_ASSET_DEFAULT;
return buildFieldPath(imageContentlet, fieldVar);
}
Logger.debug(this, "No contentlet version found for identifier: " + relatedImageId + ", returning path based on original contentlet inode: " + contentlet.getInode());
}
return buildFieldPath(contentlet, field.variable());
}

/**
* Determines whether the system should attempt to fallback to the default language
* for the given contentlet. This is used when content is not found in the
* contentlet's original language.
*
* @param contentlet The contentlet to check for language fallback eligibility
* @return true if the system should attempt to use the default language, false otherwise
*/
private boolean shouldFallbackToDefaultLanguage(final Contentlet contentlet) {
return APILocator.getLanguageAPI().canDefaultFileToDefaultLanguage() &&
APILocator.getLanguageAPI().getDefaultLanguage().getId() != contentlet.getLanguageId();
}

/**
* Retrieves the appropriate contentlet version (live or working) for an image
* based on the provided version info.
*
* @param versionInfo The version information for the contentlet
* @param live Whether to retrieve the live version (true) or working version (false)
* @return The requested version of the contentlet
* @throws DotDataException If there's an error accessing the data
* @throws DotSecurityException If there's a security violation
*/
private Contentlet getImageContentlet(final ContentletVersionInfo versionInfo, final boolean live)
throws DotDataException, DotSecurityException {
final String inode = live ? versionInfo.getLiveInode() : versionInfo.getWorkingInode();
return APILocator.getContentletAPI().find(inode, APILocator.systemUser(), false);
}

return new StringBuilder(StringPool.FORWARD_SLASH).append(contentlet.getInode())
.append(StringPool.FORWARD_SLASH).append(field.variable()).toString();
/**
* Constructs a standardized field path for a contentlet and field variable.
* The path format is: /[contentlet-inode]/[field-variable]
*
* @param contentlet The contentlet for which to build the path
* @param fieldVar The field variable to append to the path
* @return A formatted path string
*/
private String buildFieldPath(final Contentlet contentlet, final String fieldVar) {
return StringPool.FORWARD_SLASH + contentlet.getInode() + StringPool.FORWARD_SLASH + fieldVar;
}

private void validateContentlet(final Contentlet contentlet, final boolean live, final String inode) throws DotDataException {
Expand Down
6 changes: 6 additions & 0 deletions dotcms-integration/src/test/java/com/dotcms/datagen/TestDataUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,12 @@ public static Contentlet getDotAssetLikeContentlet(final Treeable hostOrFolder)

}

@WrapInTransaction
public static Contentlet getDotAssetLikeContentlet(final boolean perist, final long languageId) {
Contentlet dotAssetContentlet = getDotAssetLikeContentlet(languageId, new FolderDataGen().nextPersisted());
return perist ? ContentletDataGen.publish(dotAssetContentlet) : dotAssetContentlet;
}

@WrapInTransaction
public static Contentlet getDotAssetLikeContentlet(long languageId,
final Treeable hostOrFolder) {
Expand Down
29 changes: 25 additions & 4 deletions ...ation/src/test/java/com/dotcms/variant/business/web/VariantWebAPIImplIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@

import static org.junit.Assert.assertEquals;

import com.dotcms.JUnit4WeldRunner;
import com.dotcms.api.web.HttpServletRequestThreadLocal;
import com.dotcms.datagen.LanguageDataGen;
import com.dotcms.datagen.TestDataUtils;
import com.dotcms.datagen.VariantDataGen;
import com.dotcms.datagen.*;
import com.dotcms.mock.request.MockSession;
import com.dotcms.util.IntegrationTestInitService;
import com.dotcms.variant.VariantAPI;
import com.dotcms.variant.model.Variant;
import com.dotmarketing.business.APILocator;
import com.dotmarketing.business.web.WebAPILocator;

import javax.enterprise.context.ApplicationScoped;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

Expand All @@ -21,10 +22,10 @@
import com.dotmarketing.portlets.languagesmanager.model.Language;
import com.dotmarketing.util.PageMode;
import com.liferay.portal.model.User;
import net.bytebuddy.utility.RandomString;
import org.apache.commons.lang.RandomStringUtils;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;

import static org.junit.Assert.assertNotNull;
import static org.mockito.Mockito.mock;
Expand All @@ -33,6 +34,8 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ApplicationScoped
@RunWith(JUnit4WeldRunner.class)
public class VariantWebAPIImplIntegrationTest {

@BeforeClass
Expand Down Expand Up @@ -273,4 +276,22 @@ public void test_getContentletVersionInfoByFallback_should_get_default_content()

assertNotNull(cvi);
}


/**
* Method to test {@link VariantWebAPIImpl#getContentletVersionInfoByFallback(long, String, PageMode, User, boolean)}
* When: The dotAsset contentlet does not have a version for the language
* Should: Return the default contentlet
* @throws DotDataException
*/
@Test
public void test_getContentletVersionInfoByFallback_should_get_default_content_for_dotAsset() throws DotDataException {
final Contentlet content = TestDataUtils.getDotAssetLikeContentlet(true, APILocator.getLanguageAPI().getDefaultLanguage().getId());
final Language language = new LanguageDataGen().nextPersisted();

final VariantWebAPI variantWebAPI = WebAPILocator.getVariantWebAPI();
ContentletVersionInfo cvi = variantWebAPI.getContentletVersionInfoByFallback(language.getId(), content.getIdentifier(), PageMode.LIVE, APILocator.getUserAPI().getAnonymousUser());

assertNotNull(cvi);
}
}
94 changes: 75 additions & 19 deletions ...s-integration/src/test/java/com/dotmarketing/servlets/ShortyServletAndTitleImageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@

import java.util.Optional;

import com.dotcms.concurrent.DotSubmitter;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.image.focalpoint.FocalPoint;
import com.dotcms.contenttype.model.type.DotAssetContentType;
import com.dotcms.datagen.*;
import com.dotcms.junit.CustomDataProviderRunner;

import com.dotmarketing.portlets.languagesmanager.model.Language;
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -23,9 +23,6 @@
import com.dotcms.contenttype.model.field.ImmutableBinaryField;
import com.dotcms.contenttype.model.field.ImmutableImageField;
import com.dotcms.contenttype.model.type.ContentType;
import com.dotcms.datagen.ContentTypeDataGen;
import com.dotcms.datagen.FileAssetDataGen;
import com.dotcms.datagen.FolderDataGen;
import com.dotcms.util.IntegrationTestInitService;
import com.dotmarketing.beans.Host;
import com.dotmarketing.business.APILocator;
Expand All @@ -38,10 +35,12 @@
import com.liferay.portal.model.User;
import org.junit.runner.RunWith;

import javax.enterprise.context.ApplicationScoped;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

@RunWith(DataProviderRunner.class)
@ApplicationScoped
@RunWith(CustomDataProviderRunner.class)
public class ShortyServletAndTitleImageTest {


Expand Down Expand Up @@ -266,8 +265,6 @@ public void test_ShortyServlet_Returns_Proper_Field() throws Exception{
path.contains("/" + cvi.get().getLiveInode()));
assertTrue(path.contains("/fileAsset"));





// null out image field
Expand All @@ -284,14 +281,73 @@ public void test_ShortyServlet_Returns_Proper_Field() throws Exception{

assertTrue("shorty points to the inode", path.contains("/" + contentlet.getInode()));
assertTrue("titleImage points to binary3", path.contains("/" + binary3.variable()));








}





/**
* Method to test: {@link ShortyServlet#inodePath(Contentlet, String, boolean)}
* Given Scenario: A contentlet with an image field referencing a dot asset is created in a secondary language,
* while the actual asset only exists in the default language
* ExpectedResult: The servlet should:
* - Detect that the asset doesn't exist in the secondary language
* - Fall back to the default language version
* - Return the correct path for both live and working versions
* - Include the correct field variable (DotAssetContentType.ASSET_FIELD_VAR)
*/
@Test
public void test_inodePath_ImageField_ShouldFallbackToDefaultLanguage() throws Exception {
// Initialize servlet
final ShortyServlet servlet = new ShortyServlet();

// Create and publish dot asset in default language
Contentlet dotAssset = TestDataUtils.getDotAssetLikeContentlet(true,
APILocator.getLanguageAPI().getDefaultLanguage().getId());
APILocator.getContentletAPI().publish(dotAssset, user, false);

// Create content type with image field
final ContentType contentType = new ContentTypeDataGen().nextPersisted();
Field imageField = ImmutableImageField.builder()
.contentTypeId(contentType.id())
.name("image")
.variable("image").build();
imageField = APILocator.getContentTypeFieldAPI().save(imageField, user);

// Setup languages
final Language defaultLanguage = APILocator.getLanguageAPI().getDefaultLanguage();
final Language language2 = new LanguageDataGen().nextPersisted();

// Create contentlet in secondary language
Contentlet contentlet = new Contentlet();
contentlet.setContentTypeId(contentType.id());
contentlet.setStringProperty(imageField, dotAssset.getIdentifier());
contentlet.setLanguageId(language2.getId());
contentlet = APILocator.getContentletAPI().checkin(contentlet, user, false);

// Verify asset doesn't exist in secondary language
Optional<ContentletVersionInfo> cvi = APILocator.getVersionableAPI()
.getContentletVersionInfo(contentlet.getStringProperty(imageField.variable()),
language2.getId());
assertTrue(cvi.isEmpty());

// Verify asset exists in default language
Optional<ContentletVersionInfo> cvi2 = APILocator.getVersionableAPI()
.getContentletVersionInfo(contentlet.getStringProperty(imageField.variable()),
defaultLanguage.getId());
assertTrue(cvi2.isPresent());
assertEquals(cvi2.get().getLang(), defaultLanguage.getId());

// Verify live version path resolution
String path = servlet.inodePath(contentlet, imageField.variable(), true);
assertTrue("shorty points to the asset live inode", path.contains("/" + cvi2.get().getLiveInode()));
assertTrue(path.contains("/" + DotAssetContentType.ASSET_FIELD_VAR));

// Verify working version path resolution
path = servlet.inodePath(contentlet, imageField.variable(), false);
assertTrue("shorty points to the asset working inode", path.contains("/" + cvi2.get().getWorkingInode()));
assertTrue(path.contains("/" + DotAssetContentType.ASSET_FIELD_VAR));
}

/**
Expand Down

0 comments on commit 824106f

Please sign in to comment.