Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SITES-24155 - Content fragment list order by error #2842

Merged
merged 1 commit into from
Sep 3, 2024
Merged

SITES-24155 - Content fragment list order by error #2842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 40 additions & 7 deletions ...be/cq/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,25 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
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;
import org.apache.sling.api.resource.ResourceResolver;
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;
Expand Down Expand Up @@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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<DAMContentFragment> items = new ArrayList<>();

@PostConstruct
Expand Down Expand Up @@ -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);
}
Expand All @@ -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());
Expand Down Expand Up @@ -209,4 +219,27 @@ public Collection<DAMContentFragment> getListItems() {
public String getExportedType() {
return slingHttpServletRequest.getResource().getResourceType();
}

class ContentFragmentPredicateEvaluator extends AbstractPredicateEvaluator {
private final Comparator<String> stringComparator;

public ContentFragmentPredicateEvaluator(Locale locale) {
Collator collator = Collator.getInstance(locale);
this.stringComparator = Comparator.nullsLast(collator);
}

@Override
public Comparator<Row> 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;
}
}
}
}
8 changes: 8 additions & 0 deletions ...q/wcm/core/components/internal/models/v1/contentfragment/AbstractContentFragmentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<T> {

Expand Down Expand Up @@ -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());
Expand Down
50 changes: 49 additions & 1 deletion ...q/wcm/core/components/internal/models/v1/contentfragment/ContentFragmentListImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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"));


Expand All @@ -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<Row> comparator = pe.getOrderByComparator(null, null);
assertNotNull(comparator);

ArrayList<Row> 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
Expand Down
Loading