From 88de9975b4c466237730d5b6be3592044348ab2f Mon Sep 17 00:00:00 2001 From: levente Date: Wed, 28 Aug 2024 12:50:57 +0300 Subject: [PATCH] SITES-24155 - Content fragment list order by error * added collator based sorting on the query for content fragments * extended unit tests --- .../ContentFragmentListImpl.java | 47 ++++++++++++++--- .../AbstractContentFragmentTest.java | 8 +++ .../ContentFragmentListImplTest.java | 50 ++++++++++++++++++- 3 files changed, 97 insertions(+), 8 deletions(-) diff --git a/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImpl.java b/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImpl.java index 33dab43d15..17f035abd7 100644 --- a/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImpl.java +++ b/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImpl.java @@ -15,11 +15,17 @@ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ package com.adobe.cq.wcm.core.components.internal.models.v1.contentfragment; +import java.text.Collator; import java.util.*; import javax.annotation.PostConstruct; +import javax.jcr.RepositoryException; import javax.jcr.Session; +import javax.jcr.query.Row; +import com.day.cq.search.eval.AbstractPredicateEvaluator; +import com.day.cq.search.eval.EvaluationContext; +import com.day.cq.wcm.api.Page; import org.apache.commons.lang3.StringUtils; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.resource.Resource; @@ -27,11 +33,7 @@ import org.apache.sling.models.annotations.Default; import org.apache.sling.models.annotations.Exporter; import org.apache.sling.models.annotations.Model; -import org.apache.sling.models.annotations.injectorspecific.InjectionStrategy; -import org.apache.sling.models.annotations.injectorspecific.OSGiService; -import org.apache.sling.models.annotations.injectorspecific.Self; -import org.apache.sling.models.annotations.injectorspecific.SlingObject; -import org.apache.sling.models.annotations.injectorspecific.ValueMapValue; +import org.apache.sling.models.annotations.injectorspecific.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; @@ -63,7 +65,6 @@ ) @Exporter(name = ExporterConstants.SLING_MODEL_EXPORTER_NAME, extensions = ExporterConstants.SLING_MODEL_EXTENSION) public class ContentFragmentListImpl extends AbstractComponentImpl implements ContentFragmentList { - private static final Logger LOG = LoggerFactory.getLogger(ContentFragmentListImpl.class); public static final String RESOURCE_TYPE_V1 = "core/wcm/components/contentfragmentlist/v1/contentfragmentlist"; @@ -72,6 +73,7 @@ public class ContentFragmentListImpl extends AbstractComponentImpl implements Co public static final String DEFAULT_DAM_PARENT_PATH = "/content/dam"; public static final int DEFAULT_MAX_ITEMS = -1; + public static final String CF_COMPARATOR_REF = "ContentFragmentComparatorRef"; @Self(injectionStrategy = InjectionStrategy.REQUIRED) private SlingHttpServletRequest slingHttpServletRequest; @@ -110,6 +112,9 @@ public class ContentFragmentListImpl extends AbstractComponentImpl implements Co @Default(values = Predicate.SORT_ASCENDING) private String sortOrder; + @ScriptVariable + protected Page currentPage; + private final List items = new ArrayList<>(); @PostConstruct @@ -144,7 +149,7 @@ private void initModel() { queryParameterMap.put("1_property.value", modelPath); if (StringUtils.isNotEmpty(orderBy)) { - queryParameterMap.put("orderby", "@" + orderBy); + queryParameterMap.put("orderby", CF_COMPARATOR_REF); if (StringUtils.isNotEmpty(sortOrder)) { queryParameterMap.put("orderby.sort", sortOrder); } @@ -169,6 +174,11 @@ private void initModel() { PredicateGroup predicateGroup = PredicateGroup.create(queryParameterMap); Query query = queryBuilder.createQuery(predicateGroup, session); + if (StringUtils.isNotEmpty(orderBy)) { + Locale locale = currentPage != null ? currentPage.getLanguage(false) : Locale.getDefault(); + query.registerPredicateEvaluator(CF_COMPARATOR_REF, new ContentFragmentPredicateEvaluator(locale)); + } + SearchResult searchResult = query.getResult(); LOG.debug("Query statement: '{}'", searchResult.getQueryStatement()); @@ -209,4 +219,27 @@ public Collection getListItems() { public String getExportedType() { return slingHttpServletRequest.getResource().getResourceType(); } + + class ContentFragmentPredicateEvaluator extends AbstractPredicateEvaluator { + private final Comparator stringComparator; + + public ContentFragmentPredicateEvaluator(Locale locale) { + Collator collator = Collator.getInstance(locale); + this.stringComparator = Comparator.nullsLast(collator); + } + + @Override + public Comparator getOrderByComparator(Predicate predicate, EvaluationContext context) { + return (row1, row2) -> stringComparator.compare(getString(row1), getString(row2)); + } + + private String getString(Row row) { + try { + String str = row.getNode().getProperty(orderBy).getString(); + return StringUtils.isBlank(str) ? null : str; + } catch (RepositoryException e) { + return null; + } + } + } } diff --git a/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/AbstractContentFragmentTest.java b/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/AbstractContentFragmentTest.java index f52478ea27..e343efe20d 100644 --- a/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/AbstractContentFragmentTest.java +++ b/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/AbstractContentFragmentTest.java @@ -17,7 +17,9 @@ import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; +import java.util.Locale; +import com.day.cq.wcm.api.Page; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.resource.ValueMap; @@ -33,7 +35,9 @@ import com.day.cq.search.QueryBuilder; import io.wcm.testing.mock.aem.junit5.AemContext; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public abstract class AbstractContentFragmentTest { @@ -148,6 +152,10 @@ T getModelInstanceUnderTest(String resourceName) { slingBindings.put(SlingBindings.RESOLVER, resourceResolver); slingBindings.put(SlingBindings.RESOURCE, resource); slingBindings.put(WCMBindings.PROPERTIES, resource.adaptTo(ValueMap.class)); + Page currentPage = mock(Page.class); + when(currentPage.getLanguage(anyBoolean())).thenReturn(Locale.getDefault()); + when(currentPage.getContentResource()).thenReturn(resource); + slingBindings.put(WCMBindings.CURRENT_PAGE, currentPage); httpServletRequest.setAttribute(SlingBindings.class.getName(), slingBindings); httpServletRequest.setContextPath(CONTEXT_PATH); return httpServletRequest.adaptTo(getClassType()); diff --git a/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImplTest.java b/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImplTest.java index 072db5d3d7..cd6e3dd914 100644 --- a/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImplTest.java +++ b/bundles/core/src/test/java/com/adobe/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImplTest.java @@ -15,11 +15,19 @@ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ package com.adobe.cq.wcm.core.components.internal.models.v1.contentfragment; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; +import java.util.Locale; import java.util.Map; +import javax.jcr.Node; +import javax.jcr.Property; import javax.jcr.Session; +import javax.jcr.query.Row; +import com.day.cq.search.eval.PredicateEvaluator; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; import org.junit.jupiter.api.BeforeEach; @@ -39,6 +47,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.when; @ExtendWith(AemContextExtension.class) @@ -166,7 +175,7 @@ void verifyQueryBuilderInteractionWhenOrderByIsGiven() { "property", "jcr:content/data/cq:model", "value", "foobar")); expectedPredicates.put("orderby", ImmutableMap.of( - "orderby", "@main", + "orderby", ContentFragmentListImpl.CF_COMPARATOR_REF, "sort", "desc")); @@ -177,6 +186,45 @@ void verifyQueryBuilderInteractionWhenOrderByIsGiven() { verifyPredicateGroup(expectedPredicates, DEFAULT_NO_MAX_LIMIT_SET); } + @Test + void verifyPredicateEvaluator() { + // GIVEN + // WHEN + ContentFragmentListImpl underTest = (ContentFragmentListImpl) getModelInstanceUnderTest(MODEL_ORDER_BY); + Locale locale = Locale.US; + + // THEN + PredicateEvaluator pe = underTest.new ContentFragmentPredicateEvaluator(locale); + Comparator comparator = pe.getOrderByComparator(null, null); + assertNotNull(comparator); + + ArrayList rows = new ArrayList<>(); + Row r1 = createRow("abc"); + rows.add(r1); + Row r2 = createRow("def"); + rows.add(r2); + Row r3 = createRow("ábc"); + rows.add(r3); + Collections.sort(rows, comparator); + assertEquals(r1, rows.get(0)); + assertEquals(r3, rows.get(1)); + assertEquals(r2, rows.get(2)); + } + + private Row createRow(String value) { + Row row = Mockito.mock(Row.class); + try { + Node node = Mockito.mock(Node.class); + Property property = Mockito.mock(Property.class); + when(property.getString()).thenReturn(value); + when(node.getProperty(anyString())).thenReturn(property); + when(row.getNode()).thenReturn(node); + } catch (Exception e) { + // ignore + } + return row; + } + @Test void verifyLeakingResourceResolverIsClosed() { // GIVEN