From cc8ea713573efe4a25b6fba8b19cf2f62457d2d1 Mon Sep 17 00:00:00 2001 From: srosse <none@none> Date: Thu, 5 Sep 2013 15:05:01 +0200 Subject: [PATCH] OO-758: fix the table search, hold the value by row and dataColumn instead of index, search all columns... --- .../ChecklistMultiSelectColumnDescriptor.java | 8 +- .../components/table/ColumnDescriptor.java | 6 ++ .../table/DefaultColumnDescriptor.java | 3 +- .../GenericObjectArrayTableDataModel.java | 8 +- .../table/MultiSelectColumnDescriptor.java | 7 +- .../table/StaticColumnDescriptor.java | 5 + .../olat/core/gui/components/table/Table.java | 33 ++++--- .../gui/components/table/TableController.java | 93 ++---------------- .../components/table/TableListProvider.java | 97 +++++++++++++++++++ 9 files changed, 153 insertions(+), 107 deletions(-) create mode 100644 src/main/java/org/olat/core/gui/components/table/TableListProvider.java diff --git a/src/main/java/de/bps/olat/modules/cl/ChecklistMultiSelectColumnDescriptor.java b/src/main/java/de/bps/olat/modules/cl/ChecklistMultiSelectColumnDescriptor.java index 7bd4f12fe78..23fcd5838cd 100644 --- a/src/main/java/de/bps/olat/modules/cl/ChecklistMultiSelectColumnDescriptor.java +++ b/src/main/java/de/bps/olat/modules/cl/ChecklistMultiSelectColumnDescriptor.java @@ -19,14 +19,11 @@ */ package de.bps.olat.modules.cl; -import java.util.List; - import org.olat.core.gui.components.table.ColumnDescriptor; import org.olat.core.gui.components.table.HrefGenerator; import org.olat.core.gui.components.table.Table; import org.olat.core.gui.render.Renderer; import org.olat.core.gui.render.StringOutput; -import org.olat.core.id.Identity; import org.olat.core.logging.AssertException; /** @@ -48,6 +45,11 @@ public class ChecklistMultiSelectColumnDescriptor implements ColumnDescriptor { this.column = column; } + @Override + public int getDataColumn() { + return column; + } + public void renderValue(StringOutput sb, int row, Renderer renderer) { // add checkbox int currentPosInModel = table.getSortedRow(row); diff --git a/src/main/java/org/olat/core/gui/components/table/ColumnDescriptor.java b/src/main/java/org/olat/core/gui/components/table/ColumnDescriptor.java index ac122d51a25..cf56e54c1c4 100644 --- a/src/main/java/org/olat/core/gui/components/table/ColumnDescriptor.java +++ b/src/main/java/org/olat/core/gui/components/table/ColumnDescriptor.java @@ -64,6 +64,12 @@ public interface ColumnDescriptor { * renderer, otherwise <code>false</code> */ boolean translateHeaderKey(); + + /** + * Return the index used to retrieve the value in the table model + * @return + */ + public int getDataColumn(); /** * @return diff --git a/src/main/java/org/olat/core/gui/components/table/DefaultColumnDescriptor.java b/src/main/java/org/olat/core/gui/components/table/DefaultColumnDescriptor.java index 976b0aaf312..214f2b231cd 100644 --- a/src/main/java/org/olat/core/gui/components/table/DefaultColumnDescriptor.java +++ b/src/main/java/org/olat/core/gui/components/table/DefaultColumnDescriptor.java @@ -282,7 +282,8 @@ public class DefaultColumnDescriptor implements ColumnDescriptor { /** * @return int */ - protected int getDataColumn() { + @Override + public int getDataColumn() { return dataColumn; } diff --git a/src/main/java/org/olat/core/gui/components/table/GenericObjectArrayTableDataModel.java b/src/main/java/org/olat/core/gui/components/table/GenericObjectArrayTableDataModel.java index 37071f0d65b..3b03827f89a 100644 --- a/src/main/java/org/olat/core/gui/components/table/GenericObjectArrayTableDataModel.java +++ b/src/main/java/org/olat/core/gui/components/table/GenericObjectArrayTableDataModel.java @@ -37,7 +37,7 @@ import java.util.List; * * @author gnaegi */ -public class GenericObjectArrayTableDataModel extends DefaultTableDataModel { +public class GenericObjectArrayTableDataModel extends DefaultTableDataModel<Object[]> { private int columnCount; @@ -45,7 +45,7 @@ public class GenericObjectArrayTableDataModel extends DefaultTableDataModel { * @param objectArrays List of Object[] containing whatever data displayable by the table * @param columnCount Number of elements withing the Object[] */ - public GenericObjectArrayTableDataModel(final List objectArrays, final int columnCount) { + public GenericObjectArrayTableDataModel(final List<Object[]> objectArrays, final int columnCount) { super(objectArrays); this.columnCount = columnCount; } @@ -64,7 +64,7 @@ public class GenericObjectArrayTableDataModel extends DefaultTableDataModel { * third lastModifiedDate */ public final Object getValueAt(final int row, final int col) { - Object[] objectArray = (Object[]) getObject(row); + Object[] objectArray = getObject(row); return objectArray[col]; } @@ -76,7 +76,7 @@ public class GenericObjectArrayTableDataModel extends DefaultTableDataModel { * @param col */ public final void setValueAt(final Object o, final int row, final int col){ - Object[] objectArray = (Object[]) getObject(row); + Object[] objectArray = getObject(row); objectArray[col] = o; } } diff --git a/src/main/java/org/olat/core/gui/components/table/MultiSelectColumnDescriptor.java b/src/main/java/org/olat/core/gui/components/table/MultiSelectColumnDescriptor.java index c6d1de6b567..49dd905716f 100644 --- a/src/main/java/org/olat/core/gui/components/table/MultiSelectColumnDescriptor.java +++ b/src/main/java/org/olat/core/gui/components/table/MultiSelectColumnDescriptor.java @@ -40,7 +40,12 @@ class MultiSelectColumnDescriptor implements ColumnDescriptor { MultiSelectColumnDescriptor(){ //package visibility for constructor } - + + @Override + public int getDataColumn() { + return -1; + } + @Override public void renderValue(final StringOutput sb, final int row, final Renderer renderer) { // add checkbox diff --git a/src/main/java/org/olat/core/gui/components/table/StaticColumnDescriptor.java b/src/main/java/org/olat/core/gui/components/table/StaticColumnDescriptor.java index 047082779d9..b93e5ac1283 100644 --- a/src/main/java/org/olat/core/gui/components/table/StaticColumnDescriptor.java +++ b/src/main/java/org/olat/core/gui/components/table/StaticColumnDescriptor.java @@ -86,6 +86,11 @@ public class StaticColumnDescriptor implements ColumnDescriptor { this.translateHeaderKey = translateHeaderKey; } + @Override + public int getDataColumn() { + return -1; + } + /** * @see org.olat.core.gui.components.table.ColumnDescriptor#getAlignment() */ diff --git a/src/main/java/org/olat/core/gui/components/table/Table.java b/src/main/java/org/olat/core/gui/components/table/Table.java index cf8a1545790..b1a6888819a 100644 --- a/src/main/java/org/olat/core/gui/components/table/Table.java +++ b/src/main/java/org/olat/core/gui/components/table/Table.java @@ -37,6 +37,7 @@ import org.olat.core.gui.components.Component; import org.olat.core.gui.components.ComponentRenderer; import org.olat.core.gui.control.Event; import org.olat.core.gui.render.StringOutput; +import org.olat.core.gui.render.StringOutputPool; import org.olat.core.gui.translator.Translator; import org.olat.core.logging.OLATRuntimeException; import org.olat.core.logging.OLog; @@ -176,6 +177,10 @@ public class Table extends Component { protected ColumnDescriptor getColumnDescriptorFromAllCDs(final int column) { return allCDs.get(column); } + + protected int getColumnCountFromAllCDs() { + return allCDs.size(); + } /** * @return Column descriptor of currently sorted column @@ -885,14 +890,14 @@ public class Table extends Component { } } - private void buildFilteredTableDataModel(final String tableSearchString2) { + private void buildFilteredTableDataModel(final String searchString) { List<Object> filteredElementList = new ArrayList<Object>(); log.debug("buildFilteredTableDataModel: tableDataModel.getRowCount()=" + tableDataModel.getRowCount()); if (tableDataModel.getRowCount() > 0) { log.debug("buildFilteredTableDataModel: tableDataModel.getObject(0)=" + tableDataModel.getObject(0)); } for (int row = 0; row < tableDataModel.getRowCount(); row++) { - if (matchRowWithSearchString(row, tableSearchString2)) { + if (matchRowWithSearchString(row, searchString)) { filteredElementList.add(tableDataModel.getObject(row)); } } @@ -912,14 +917,18 @@ public class Table extends Component { return true; } // loop over all columns + TableDataModel unfilteredModel = getUnfilteredTableDataModel(); + Filter htmlFilter = FilterFactory.getHtmlTagsFilter(); - for (int colIndex = 0; colIndex < getUnfilteredTableDataModel().getColumnCount(); colIndex++) { - Object value = getUnfilteredTableDataModel().getValueAt(row, colIndex); - // When a CustomCellRenderer exist, use this to render cell-value to String - ColumnDescriptor cd = this.getColumnDescriptorFromAllCDs(colIndex); - if (isColumnDescriptorVisible(cd)) { + for (int colIndex = getColumnCountFromAllCDs(); colIndex-->0; ) { + ColumnDescriptor cd = getColumnDescriptorFromAllCDs(colIndex); + int dataColumn = cd.getDataColumn(); + if (dataColumn >= 0 && isColumnDescriptorVisible(cd)) { + Object value = unfilteredModel.getValueAt(row, dataColumn); + // When a CustomCellRenderer exist, use this to render cell-value to String if (cd instanceof CustomRenderColumnDescriptor) { - CustomCellRenderer customCellRenderer = ((CustomRenderColumnDescriptor)cd).getCustomCellRenderer(); + CustomRenderColumnDescriptor cdrd = (CustomRenderColumnDescriptor)cd; + CustomCellRenderer customCellRenderer = cdrd.getCustomCellRenderer(); if (customCellRenderer instanceof CustomCssCellRenderer) { // For css renderers only use the hover // text, not the CSS class name and other @@ -930,9 +939,9 @@ public class Table extends Component { continue; } } else { - StringOutput sb = new StringOutput(100); - customCellRenderer.render(sb, null, value, ((CustomRenderColumnDescriptor) cd).getLocale(), cd.getAlignment(), null); - value = sb.toString(); + StringOutput sb = StringOutputPool.allocStringBuilder(250); + customCellRenderer.render(sb, null, value, cdrd.getLocale(), cd.getAlignment(), null); + value = StringOutputPool.freePop(sb); } } @@ -940,10 +949,8 @@ public class Table extends Component { String valueAsString = (String)value; // Remove any HTML markup from the value valueAsString = htmlFilter.filter(valueAsString); - log.debug("matchRowWithFilter: check " + valueAsString); // Finally compare with search value based on a simple lowercase match if (valueAsString.toLowerCase().indexOf(tableSearchString2.toLowerCase()) != -1 ) { - log.debug("matchRowWithFilter: found match for row=" + row + " value=" + valueAsString + " with filter=" + tableSearchString2); return true; } } diff --git a/src/main/java/org/olat/core/gui/components/table/TableController.java b/src/main/java/org/olat/core/gui/components/table/TableController.java index c9126611365..ea2d9193e37 100644 --- a/src/main/java/org/olat/core/gui/components/table/TableController.java +++ b/src/main/java/org/olat/core/gui/components/table/TableController.java @@ -31,8 +31,6 @@ import java.util.BitSet; import java.util.Collections; import java.util.Iterator; import java.util.List; -import java.util.Set; -import java.util.TreeSet; import org.olat.core.gui.ShortName; import org.olat.core.gui.UserRequest; @@ -48,19 +46,14 @@ import org.olat.core.gui.control.controller.BasicController; import org.olat.core.gui.control.generic.ajax.autocompletion.AutoCompleterController; import org.olat.core.gui.control.generic.ajax.autocompletion.EntriesChosenEvent; import org.olat.core.gui.control.generic.ajax.autocompletion.ListProvider; -import org.olat.core.gui.control.generic.ajax.autocompletion.ListReceiver; import org.olat.core.gui.control.generic.closablewrapper.CloseableCalloutWindowController; import org.olat.core.gui.media.MediaResource; -import org.olat.core.gui.render.StringOutput; import org.olat.core.gui.translator.PackageTranslator; import org.olat.core.gui.translator.Translator; import org.olat.core.logging.AssertException; import org.olat.core.logging.OLog; import org.olat.core.logging.Tracing; -import org.olat.core.util.StringHelper; import org.olat.core.util.Util; -import org.olat.core.util.filter.Filter; -import org.olat.core.util.filter.FilterFactory; /** * <!--**************--> @@ -121,8 +114,6 @@ public class TableController extends BasicController { private static final String VC_VAR_HAS_TABLE_SEARCH = "hasTableSearch"; - private static final String LOG_DEBUG_DURATION = " duration="; - private OLog log = Tracing.createLoggerFor(this.getClass()); private static final String CMD_FILTER = "cmd.filter."; @@ -136,10 +127,6 @@ public class TableController extends BasicController { */ public static final Event EVENT_FILTER_SELECTED = new Event("filter.selected"); - /** - * Limit the number of search-suggestions in table-search-popup - */ - private static final int MAX_TABLE_SEARCH_RESULT_ENTRIES = 20; private VelocityContainer contentVc; @@ -303,69 +290,7 @@ public class TableController extends BasicController { } private Controller createTableSearchController(final UserRequest ureq, final WindowControl wControl) { - ListProvider genericProvider = new ListProvider() { - public void getResult(final String searchValue, final ListReceiver receiver) { - Filter htmlFilter = FilterFactory.getHtmlTagsFilter(); - log.debug("getResult start"); - long startTime = System.currentTimeMillis(); - Set<String> searchEntries = new TreeSet<String>(); - int entryCounter = 1; - // loop over whole data-model - for (int rowIndex=0; rowIndex < table.getUnfilteredTableDataModel().getRowCount(); rowIndex++) { - for (int colIndex=0; colIndex < table.getUnfilteredTableDataModel().getColumnCount(); colIndex++) { - Object obj = table.getUnfilteredTableDataModel().getValueAt(rowIndex, colIndex); - // When a CustomCellRenderer exist, use this to render cell-value to String - ColumnDescriptor cd = table.getColumnDescriptorFromAllCDs(colIndex); - if (table.isColumnDescriptorVisible(cd)) { - - if (cd instanceof CustomRenderColumnDescriptor) { - CustomCellRenderer customCellRenderer = ((CustomRenderColumnDescriptor)cd).getCustomCellRenderer(); - if (customCellRenderer instanceof CustomCssCellRenderer) { - // For css renderers only use the hover - // text, not the CSS class name and other - // markup - CustomCssCellRenderer cssRenderer = (CustomCssCellRenderer) customCellRenderer; - obj = cssRenderer.getHoverText(obj); - if (!StringHelper.containsNonWhitespace((String) obj)) { - continue; - } - } else { - StringOutput sb = new StringOutput(); - customCellRenderer.render(sb, null, obj, ((CustomRenderColumnDescriptor) cd).getLocale(), cd.getAlignment(), null); - obj = sb.toString(); - } - } - - if (obj instanceof String) { - String valueString = (String)obj; - // Remove any HTML markup from the value - valueString = htmlFilter.filter(valueString); - // Finally compare with search value based on a simple lowercase match - if (valueString.toLowerCase().indexOf(searchValue.toLowerCase()) != -1) { - if (searchEntries.add(valueString) ) { - // Add to receiver list same entries only once - if (searchEntries.size() == 1) { - // before first entry, add searchValue. But add only when one search match - receiver.addEntry( searchValue, searchValue ); - } - // limit the number of entries - if (entryCounter++ > MAX_TABLE_SEARCH_RESULT_ENTRIES) { - receiver.addEntry("...", "..."); - long duration = System.currentTimeMillis() - startTime; - log.debug("getResult reach MAX_TABLE_SEARCH_RESULT_ENTRIES, entryCounter=" + entryCounter + LOG_DEBUG_DURATION + duration); - return; - } - receiver.addEntry(valueString, valueString); - } - } - } - } - } - } - long duration = System.currentTimeMillis() - startTime; - log.debug("getResult finished entryCounter=" + entryCounter + LOG_DEBUG_DURATION + duration); - } - }; + ListProvider genericProvider = new TableListProvider(table); removeAsListenerAndDispose(tableSearchController); tableSearchController = new AutoCompleterController(ureq, wControl, genericProvider, null, false, 60, 3, translate("table.filter.label")); listenTo(tableSearchController); // TODO:CG 02.09.2010 Test Tablesearch Performance, remove @@ -431,7 +356,7 @@ public class TableController extends BasicController { } private void applyAndcheckChangedColumnsChoice(final UserRequest ureq) { - List selRows = colsChoice.getSelectedRows(); + List<Integer> selRows = colsChoice.getSelectedRows(); if (selRows.size() == 0) { showError("error.selectatleastonecolumn"); } else { @@ -463,17 +388,18 @@ public class TableController extends BasicController { return choice; } + @Override public void event(final UserRequest ureq, final Controller source, final Event event) { log.debug("dispatchEvent event=" + event + " source=" + source); if (event instanceof EntriesChosenEvent) { EntriesChosenEvent ece = (EntriesChosenEvent)event; - List filterList = ece.getEntries(); + List<String> filterList = ece.getEntries(); if (!filterList.isEmpty()) { - this.table.setSearchString((String)filterList.get(0)); - this.modelChanged(false); + table.setSearchString(filterList.get(0)); + modelChanged(false); } else { // reset filter search filter in modelChanged - this.modelChanged(); + modelChanged(); } } } @@ -493,9 +419,7 @@ public class TableController extends BasicController { */ public Object getSortedObjectAt(int sortedRow) { int row = table.getSortedRow(sortedRow); - TableDataModel model = getTableDataModel(); - Object obj = model.getObject(row); - return obj; + return getTableDataModel().getObject(row); } /** @@ -780,5 +704,4 @@ public class TableController extends BasicController { protected void doDispose() { // } - } diff --git a/src/main/java/org/olat/core/gui/components/table/TableListProvider.java b/src/main/java/org/olat/core/gui/components/table/TableListProvider.java new file mode 100644 index 00000000000..9ec93354c43 --- /dev/null +++ b/src/main/java/org/olat/core/gui/components/table/TableListProvider.java @@ -0,0 +1,97 @@ +package org.olat.core.gui.components.table; + +import java.util.Set; +import java.util.TreeSet; + +import org.olat.core.gui.control.generic.ajax.autocompletion.ListProvider; +import org.olat.core.gui.control.generic.ajax.autocompletion.ListReceiver; +import org.olat.core.gui.render.StringOutput; +import org.olat.core.gui.render.StringOutputPool; +import org.olat.core.util.StringHelper; +import org.olat.core.util.filter.Filter; +import org.olat.core.util.filter.FilterFactory; + +/** + * + * Initial date: 05.09.2013<br> + * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com + * + */ +public class TableListProvider implements ListProvider { + + /** + * Limit the number of search-suggestions in table-search-popup + */ + private static final int MAX_TABLE_SEARCH_RESULT_ENTRIES = 20; + + private final Table table; + + public TableListProvider(Table table) { + this.table = table; + } + + @Override + public void getResult(String searchValue, ListReceiver receiver) { + Filter htmlFilter = FilterFactory.getHtmlTagsFilter(); + Set<String> searchEntries = new TreeSet<String>(); + int entryCounter = 1; + // loop over whole data-model + + TableDataModel<?> unfilteredModel = table.getUnfilteredTableDataModel(); + + int rowCount = unfilteredModel.getRowCount(); + int colCount = table.getColumnCountFromAllCDs(); + + a_a: + for (int colIndex=0; colIndex < colCount; colIndex++) { + ColumnDescriptor cd = table.getColumnDescriptorFromAllCDs(colIndex); + int dataColumn = cd.getDataColumn(); + if (dataColumn >= 0 && table.isColumnDescriptorVisible(cd)) { + for (int rowIndex=0; rowIndex < rowCount; rowIndex++) { + Object obj = unfilteredModel.getValueAt(rowIndex, dataColumn); + // When a CustomCellRenderer exist, use this to render cell-value to String + if (cd instanceof CustomRenderColumnDescriptor) { + CustomRenderColumnDescriptor crcd = (CustomRenderColumnDescriptor)cd; + CustomCellRenderer customCellRenderer = crcd.getCustomCellRenderer(); + if (customCellRenderer instanceof CustomCssCellRenderer) { + // For css renderers only use the hover + // text, not the CSS class name and other + // markup + CustomCssCellRenderer cssRenderer = (CustomCssCellRenderer) customCellRenderer; + obj = cssRenderer.getHoverText(obj); + if (!StringHelper.containsNonWhitespace((String) obj)) { + continue; + } + } else { + StringOutput sb = StringOutputPool.allocStringBuilder(250); + customCellRenderer.render(sb, null, obj, crcd.getLocale(), cd.getAlignment(), null); + obj = StringOutputPool.freePop(sb); + } + } + + if (obj instanceof String) { + String valueString = (String)obj; + // Remove any HTML markup from the value + valueString = htmlFilter.filter(valueString); + // Finally compare with search value based on a simple lowercase match + if (valueString.toLowerCase().indexOf(searchValue.toLowerCase()) != -1) { + if (searchEntries.add(valueString) ) { + // Add to receiver list same entries only once + if (searchEntries.size() == 1) { + // before first entry, add searchValue. But add only when one search match + receiver.addEntry( searchValue, searchValue ); + } + // limit the number of entries + if (entryCounter++ > MAX_TABLE_SEARCH_RESULT_ENTRIES) { + receiver.addEntry("...", "..."); + break a_a; + } + receiver.addEntry(valueString, valueString); + } + } + } + } + } + } + } +} \ No newline at end of file -- GitLab