Ask Questions and Find Answers

It turns out that this was only the tip of the iceberg:

To ensure that the email address is indexed, I had to add that property to my portal-ext.properties:

index.sortable.text.fields=firstName,jobTitle,lastName,name,screenName,title,emailAddress

(which I had already done in 6.2 for one of my instances - and I needed some time to find out why email addresses weren't sorted on my other instances).

Now I wanted to add some more columns, thus I improved my ViewUsersManagementToolbarDisplayContext.java:

public class ViewUsersManagementToolbarDisplayContext
        extends com.liferay.users.admin.web.internal.display.context.ViewUsersManagementToolbarDisplayContext {

    private HttpServletRequest request;

    public ViewUsersManagementToolbarDisplayContext(HttpServletRequest request, RenderRequest renderRequest,
            RenderResponse renderResponse, String displayStyle, String navigation, int status) {
        super(request, renderRequest, renderResponse, displayStyle, navigation, status);
        this.request = request;
    }

    @Override
    public List<DropdownItem> getFilterDropdownItems() {
        List<DropdownItem> items = super.getFilterDropdownItems();
        addOrderColumn(items, "email-address", "email-address");
        addOrderColumn(items, "createDate", "create-date");
        addOrderColumn(items, "lastLoginDate", "Login");
        addOrderColumn(items, "lockoutDate", "locked");
        addOrderColumn(items, "jobTitle", "job-title");
        return items;
    }

    private void addOrderColumn(List<DropdownItem> items, final String columnKey, String columnLabel) {
        DropdownGroupItem sortGroup = (DropdownGroupItem) items.get(items.size() - 1);
        DropdownItemList itemList = (DropdownItemList) sortGroup.get("items");
        itemList.add(dropdownItem -> {
            dropdownItem.setActive(Objects.equals(getOrderByCol(), columnKey));
            dropdownItem.setHref(getPortletURL(), "orderByCol", columnKey);
            dropdownItem.setLabel(LanguageUtil.get(this.request, columnLabel));
        });
    }

}

But this isn't enough, as there is a mapper function com.liferay.portlet.usersadmin.util.UsersAdminImpl.getUserOrderByComparator which maps the column names to a comparator. And that function returns the UserLastNameComparator for every unknown column, which includes all my date columns. UsersAdmin is an old fashioned Liferay service class, which means it resides in portal-impl.jar and is sometimes referenced with UsersAdminUtil and sometimes injected with @Reference.

In 6.2 I would have written a hook or ext-plugin to wrap that service in my own implementation class (and use spring-ext.xml to install it), but for 7.1 I decided to write a new plugin which contains my wrapper class, with some additional tricks from my magic bag:

@Component(immediate = true, service = UsersAdmin.class)
public class MyUsersAdminImpl implements UsersAdmin {

    private UsersAdmin wrapped;

    @Activate
    public void activate() {
        // Install our implementation on top of the existing implementation
        if (UsersAdminUtil.getUsersAdmin() instanceof MyUsersAdminImpl) {
            this.wrapped = ((MyUsersAdminImpl) UsersAdminUtil.getUsersAdmin()).getWrapped();
        } else {
            this.wrapped = UsersAdminUtil.getUsersAdmin();
        }
        new UsersAdminUtil().setUsersAdmin(this);
    }    

    @Deactivate
    public void deactivate() {
        // Reinstall the previous implementation
        if (UsersAdminUtil.getUsersAdmin() == this) {
            new UsersAdminUtil().setUsersAdmin(this.wrapped);
        }
    }

    ... all UsersAdmin functions delegation to wrapper ...

}

Now I implemented my own comparator, but instead of one per property (as done in Liferay) I like a more generic aproach:

public class GenericComparator<T> extends OrderByComparator<T> {
    private static final long serialVersionUID = 1L;

    private final boolean ascending;

    private final String orderBy;

    private final String[] orderByFields;

    private final Comparator<T> comparator;

    public GenericComparator(String field, Comparator<T> comparator) {
        this(field, comparator, true);
    }

    public GenericComparator(String field, Comparator<T> comparator, boolean ascending) {
        this(field + (ascending ? " ASC" : " DESC"), new String[] { field }, comparator, ascending);
    }

    public GenericComparator(String orderBy, String[] orderByFields, Comparator<T> comparator, boolean ascending) {
        this.orderBy = orderBy;
        this.orderByFields = orderByFields;
        this.comparator = comparator;
        this.ascending = ascending;
    }

    @Override
    public int compare(T entity1, T entity2) {
        final int value = this.comparator.compare(entity1, entity2);
        return this.ascending ? value : -value;
    }

    @Override
    public String getOrderBy() {
        return this.orderBy;
    }

    @Override
    public String[] getOrderByFields() {
        return this.orderByFields;
    }

    @Override
    public boolean isAscending() {
        return this.ascending;
    }

}

That comparator I can use in my own getUserOrderByComparator:

@Component(immediate = true, service = UsersAdmin.class)
public class MyUsersAdminImpl implements UsersAdmin {

    ...

    @Override
    public OrderByComparator<User> getUserOrderByComparator(String orderByCol, String orderByType) {
        // Support the default comparators
        if (orderByCol.indexOf('-') >= 0) {
            return this.wrapped.getUserOrderByComparator(orderByCol, orderByType);
        }

        // Support additional (generic) comparators
        final boolean orderByAsc = orderByType.equals("asc");
        try {
            final Method getter = User.class.getMethod("get" + StringUtil.upperCaseFirstLetter(orderByCol));
            return new GenericComparator<>(orderByCol, (a, b) -> {
                try {
                    return ((Comparable<Object>) getter.invoke(a)).compareTo(getter.invoke(b));
                } catch (IllegalAccessException | InvocationTargetException e) {
                    throw new IllegalArgumentException(e);
                }
            }, orderByAsc);
        } catch (NoSuchMethodException | SecurityException e) {
            return this.wrapped.getUserOrderByComparator(orderByCol, orderByType);
        }
    }

}

And this isn't still enough, because all the date columns are not indexed. Unfortunately I can't add these to index.sortable.text.fields from above, because they are not of type text.

Thus I added a UserIndexPostProcessor to my plugin:

@Component(immediate = true, service = IndexerPostProcessor.class, 
        property = { "indexer.class.name=com.liferay.users.admin.internal.search.UserIndexer" })
public class UserIndexerPostProcessor extends BaseIndexerPostProcessor {

    public void postProcessDocument(Document document, Object obj) throws Exception {
        final User user = (User) obj;
        // Add date fields for sorting to index
        document.addDateSortable("createDate", user.getCreateDate());
        document.addDateSortable("lastLoginDate", user.getLastLoginDate());
        document.addDateSortable("lockoutDate", user.getLockoutDate());
    }

    @Override
    public void postProcessSearchQuery(BooleanQuery searchQuery, BooleanFilter booleanFilter,
            SearchContext searchContext) throws Exception {
        // Ensure that datefields are mapped correctly
        final Sort[] sorts = searchContext.getSorts();
        if (sorts != null) {
            for (int i = 0; i < sorts.length; i++) {
                final Sort sort = sorts[i];
                if (sort.getType() == Sort.STRING_TYPE && sort.getFieldName().endsWith("Date")) {
                    sorts[i] = new Sort(sort.getFieldName() + "_Number_sortable", Sort.LONG_TYPE, sort.isReverse());
                }
            }
        }
    }

}

And after reindexing the users I finally could sort by all these new columns.

Conclusion:

I had to write a lot of code and had to use a lot of inside knowledge of Liferay - which may change already by the next patch version. Just to add some simple sortable date columns.

By stepping through the code I found many places where simle coding standards like the DRY principle were violated - with huge drawbacks for maintainability, extensibility and readability. Liferay 7.1 has about 3.2 million code lines in Java files (not counting empty / commented lines, JSP files and other templates). This is an increase by  ~ 300,000 code lines (11%) compared to Liferay 7.0. Some might say this is an indicator for more functionality. But for me this is an indicator for more sources of errors and vulnerabilities, more code nobody is aware of, more code to maintain, more code you need to overwrite for your own extensions.

Thus this post is more a call for higher code quality in Liferay than it is a tutorial on how to add sortable columns in Liferay.

Tobias Liefke:

But instead of a straight forward buildOrderByDropdownItems("first-name", "last-name, "screen-name"), there is the same boilerplate code again and again.

Just a personal thing, but I never understood why lambdas were supposed to be so much better than straight-up OOP...

Unfortunately the code just makes them all look boilerplate. Each string is used 3 times, once for the column identifier, once for the sort parameter value in the url, and once for the label to use for the value.  Since Liferay uses the same value for all 3 cases, this appears to be boilerplate because you get the same three lines using the same constant string over and over and over.  But that's just Liferay, there is the option to override and use different keys or identifiers or ...  The fact that Liferay tends to use the same value is representative of the consistency they use in their code.  The "buildOrderByDropdownItems(...)" idea would only work when all of the values would be the same, a case that is not always guaranteed.
 

Hi Tobias,

I'm sorry for all the trouble you had to go through when trying to extend Liferay functionality. I'm Liferay's Engineering Lead of Search and I agree with you that sorting on a few custom fields shouldn't be so hard and take so much code. We've been listening to similar reports and in the roadmap of our next version, sort improvements were selected as a priority currently in active development.

https://issues.liferay.com/browse/LPS-84969

I wanted to say the level of detail you provided is remarkable and will be extremely helpful in making sure we implement this feature just right. This is an immensely useful, specific, end-to-end use case and we will make sure it gets properly covered. I would love to hear any other insights you may have from your experience with customization of Liferay Search.

Thank you,

André

Tobias,

You make very valid points. We're going to fix the 1.) code duplication and 2.) make it easier to customize columns like you could back in 6.2.x. There's no reason things simple should be complicated.

The Commerce team already has a workaround they've been using for 7.1.x. See this commit.

I have a sprint in January where I will 1.) digest the problem as you've described (and which Marco Leo has brought up to me before) and 2.) bring in Marco's solution to the portal. Then, I'm going to assign someone to de-duplicate and apply the pattern every where.

We will fix this. You have my word.