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.

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

DataTable: LazyDataModel.load gets invoked multiple times when datatable is lazy and has headerRow #11452

Closed
coderoth opened this issue Feb 16, 2024 · 4 comments · Fixed by #11457
Assignees
Labels
⚡ performance Performance related issue or enhancement
Milestone

Comments

@coderoth
Copy link

Describe the bug

DataTable component invokes LazyDataModel::load multiple times (one for each item in the current page of the datatable) when the datatable is lazy and has a headerRow.

Please, check the attached zip that reproduces the problem and shows one workaround to the issue.
To execute the reproducer:

mvn jetty:run

And open the following page in the browser

http://localhost:8080/primefaces-test

primefaces-test.zip

Reproducer

The ProblematicLazyDataModel::load method below will be executed multiple times (one for each returned item in the pagination). With datatable configured with rows=5, this is the server output (showing that querying a real database would result in 5 queries being executed with weird pagination parameters):

ProblematicLazyDataModel.load(first=0, size=5) executed in the fake database
ProblematicLazyDataModel.load(first=0, size=1) executed in the fake database
ProblematicLazyDataModel.load(first=1, size=2) executed in the fake database
ProblematicLazyDataModel.load(first=2, size=3) executed in the fake database
ProblematicLazyDataModel.load(first=3, size=4) executed in the fake database

Please, check provided uploaded primefaces-test.zip for a working and complete example.

Sample xhtml:

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml"
        xmlns:h="http://xmlns.jcp.org/jsf/html"
        xmlns:f="http://xmlns.jcp.org/jsf/core"
        xmlns:pt="http://xmlns.jcp.org/jsf/passthrough"
        xmlns:jsf="http://xmlns.jcp.org/jsf"
        xmlns:ui="http://xmlns.jcp.org/jsf/facelets"
        xmlns:p="http://primefaces.org/ui">

    <h:head>
        <title>PrimeFaces Test</title>
        <h:outputScript name="test.js" />
        <h:outputStylesheet name="test.css" />
    </h:head>
    <h:body>

        <h1>Problem</h1>

                <p:dataTable value="#{problematicLazyDataModel}" var="obj"
                                         lazy="true" paginator="true"
                                         paginatorPosition="bottom" rows="5">

                        <p:headerRow field="artist" expandable="false">
                                <p:column colspan="3">#{obj.artist}</p:column>
                        </p:headerRow>

                        <p:column headerText="ID">
                                <h:outputText value="#{obj.id}" />
                        </p:column>

                        <p:column headerText="Song">
                                <h:outputText value="#{obj.name}" />
                        </p:column>

                        <p:column headerText="Released">
                                <h:outputText value="#{obj.released}" />
                        </p:column>

                </p:dataTable>

    </h:body>
</html>

Sample problematic datamodel:

package org.primefaces.test;

import java.util.List;
import java.util.Map;

import javax.faces.view.ViewScoped;
import javax.inject.Named;

import org.primefaces.model.FilterMeta;
import org.primefaces.model.LazyDataModel;
import org.primefaces.model.SortMeta;

@Named
@ViewScoped
public class ProblematicLazyDataModel extends LazyDataModel<TestObject> {

        private static final long serialVersionUID = 1L;

        @Override
        public int count(Map<String, FilterMeta> filterBy) {
                return FakeDB.count();
        }

        @Override
        public List<TestObject> load(int first, int pageSize, Map<String, SortMeta> sortBy, Map<String, FilterMeta> filterBy) {
                System.out.println(String.format("%s.load(first=%d, size=%d) executed in the fake database", this.getClass().getSimpleName(), first, pageSize));
                return FakeDB.find(first, pageSize);
        }

}

Expected behavior

DataTable should invoke the LazyDataModel::load method once, like que WorkaroundLazyDataModel example below.
The output:

WorkaroundLazyDataModel.load(first=0, size=5) executed in the fake database

WorkaroundLazyDataModel code, with overriden getRowData method (the workaround):

package org.primefaces.test;

import java.util.List;
import java.util.Map;

import javax.faces.view.ViewScoped;
import javax.inject.Named;

import org.primefaces.model.FilterMeta;
import org.primefaces.model.LazyDataModel;
import org.primefaces.model.SortMeta;

@Named
@ViewScoped
public class WorkaroundLazyDataModel extends LazyDataModel<TestObject> {

        private static final long serialVersionUID = 1L;

        @Override
        public int count(Map<String, FilterMeta> filterBy) {
                return FakeDB.count();
        }

        @Override
        public List<TestObject> load(int first, int pageSize, Map<String, SortMeta> sortBy, Map<String, FilterMeta> filterBy) {
                System.out.println(String.format("%s.load(first=%d, size=%d) executed in the fake database", this.getClass().getSimpleName(), first, pageSize));
                return FakeDB.find(first, pageSize);
        }

        /*
         * WORKAROUND
         *
         * */
        @Override
        public TestObject getRowData(int rowIndex, Map<String, SortMeta> sortBy, Map<String, FilterMeta> filterBy) {

                if (rowIndex < 0 || getPageSize() == 0) {
                        return null;
                }

                int pageRowIndex = (rowIndex % getPageSize());
                List<TestObject> wrappedData = getWrappedData();
                if (wrappedData != null && wrappedData.size() > pageRowIndex) {
                        return wrappedData.get(pageRowIndex);
                }

                // couldnt load from cache, trying the old way
                return super.getRowData(rowIndex, sortBy, filterBy);
        }

}

PrimeFaces edition

Community

PrimeFaces version

13.0.5

Theme

No response

JSF implementation

Mojarra

JSF version

2.3.21

Java version

1.8.0_361

Browser(s)

No response

@coderoth coderoth added ‼️ needs-triage Issue needs triaging 🐞 defect Bug...Something isn't working labels Feb 16, 2024
@melloware melloware removed the ‼️ needs-triage Issue needs triaging label Feb 16, 2024
@melloware
Copy link
Member

melloware commented Feb 16, 2024

It was introduced by this ticket: #9077 cc @sebsoftware

Looking at the source code it seems this was intentional with a comment in the DataTableRenderer.isInSameGroup method:

        // in case of a lazy DataTable, the LazyDataModel currently only loads rows inside the current page; we need a small hack here
        // 1) get the rowData manually for the next row
        // 2) put it into request-scope
        // 3) invoke the groupBy ValueExpression
        if (table.isLazy()) {
            Object nextRowData = table.getLazyDataModel().getRowData(nextRowIndex, table.getActiveSortMeta(), table.getActiveFilterMeta());
            if (nextRowData == null) {
                return false;
            }

            nextGroupByData = ComponentUtils.executeInRequestScope(context, table.getVar(), nextRowData, () -> groupByVE.getValue(elContext));
        }

@melloware melloware added the discussion Item needs to be discussed by core devs label Feb 16, 2024
@Rapster Rapster self-assigned this Feb 18, 2024
Rapster added a commit to Rapster/primefaces that referenced this issue Feb 18, 2024
…tiple times when datatable is lazy and has headerRow
@melloware melloware added this to the 13.0.6 milestone Feb 18, 2024
@melloware melloware removed the discussion Item needs to be discussed by core devs label Feb 18, 2024
@Rapster
Copy link
Member

Rapster commented Feb 18, 2024

Indeed, that extra call is not needed when no summaryRow present

@melloware melloware added ⚡ performance Performance related issue or enhancement and removed 🐞 defect Bug...Something isn't working labels Feb 19, 2024
melloware added a commit that referenced this issue Feb 19, 2024
#11457)

* Fix #11452 - DataTable: LazyDataModel.load gets invoked multiple times when datatable is lazy and has headerRow

* Fix and rename LazyDataModel#getRowData into LazyDataModel#loadOne to avoid confusion

* Update 14_0_0.md

* Update LazyDataModel.java

* Update DataTableRenderer.java

* Update 14_0_0.md

* Update DataTableRenderer.java

* Update LazyDataModel.java

---------

Co-authored-by: Melloware <mellowaredev@gmail.com>
melloware added a commit to melloware/primefaces that referenced this issue Feb 19, 2024
@coderoth
Copy link
Author

Wow, thanks for the fix!

@melloware
Copy link
Member

13.0.6 is in Maven Central if you want to try it @coderoth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡ performance Performance related issue or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants