Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » NatTable » Bug in FilterRowDataProvider.{save,load}State()?(Doesn't handle threshhold filters well)
Bug in FilterRowDataProvider.{save,load}State()? [message #1017380] Mon, 11 March 2013 17:59 Go to next message
Patrick Rusk is currently offline Patrick Rusk
Messages: 35
Registered: June 2012
Member
I believe FilterRowDataProvider.{save,load}State() have a bug in the way that they handle the persistence of filters. Here is the (highly edited) code:
public void saveState(String prefix, Properties properties) {
	*	*	*
        filterTextByIndex.put(columnIndex, (String) converter.canonicalToDisplayValue(filterObjectByIndex.get(columnIndex)));
	*	*	*
}

public void loadState(String prefix, Properties properties) {
	*	*	*
            filterObjectByIndex.put(columnIndex, converter.displayToCanonicalValue(filterTextByIndex.get(columnIndex)));
	*	*	*
}

The problem here is that the string used in the filter is converted to a display value on a save, then converted to canonical value on a load. That causes real problems with filters like ">500" on a numeric column. On the load, the conversion of ">500" to a number fails, and you get a message written to standard error saying "Error while restoring filter row text: ...".

Is there any good reason why the string isn't just saved and loaded "as is"?

This is a tough problem to fix without modifying the original source (like by creating a subclass of FilterRowDataProvider) because FilterRowDataProvider is referenced explicitly in FilterRowDataLayer, which is mentioned explicitly in FilterRowHeaderComposite.

[Updated on: Mon, 11 March 2013 18:02]

Report message to a moderator

Re: Bug in FilterRowDataProvider.{save,load}State()? [message #1017609 is a reply to message #1017380] Tue, 12 March 2013 07:39 Go to previous messageGo to next message
Dirk Fauth is currently offline Dirk Fauth
Messages: 1301
Registered: July 2012
Senior Member
Quote:
Is there any good reason why the string isn't just saved and loaded "as is"?


Of course there is a good reason. If you registered a converter to the filter row cell, e.g. an Integer converter, then how should a String for the matcher be restored to match an Integer?

Quote:
This is a tough problem to fix without modifying the original source


You know that we are an open source project? If you find a bug and know how to fix it, try to contribute a bugfix via bugzilla and patch. Wink

I will take a look at this and tell you what I've found out.
Re: Bug in FilterRowDataProvider.{save,load}State()? [message #1017947 is a reply to message #1017609] Tue, 12 March 2013 20:41 Go to previous messageGo to next message
Patrick Rusk is currently offline Patrick Rusk
Messages: 35
Registered: June 2012
Member
Quote:
If you registered a converter to the filter row cell, e.g. an Integer converter, then how should a String for the matcher be restored to match an Integer?

Users type sequences of characters into filter cells, and the converter for that cell converts them into the Integer that it internally wants. Therefore, I would have expected that when the string is restored to the filter cell, the converter itself would convert it at that point.
Quote:
You know that we are an open source project? If you find a bug and know how to fix it, try to contribute a bugfix via bugzilla and patch. Wink

Heh. I have not always seen consistency in open source projects regarding bugs. Some want them discussed on forums until a committer says, "Add it as a bug", and some want them added as bugs right away. What is the convention for NatTable? I am glad to comply.

As for contributing a fix, I would have a steep learning curve to get to the point where I could do that (having not used Git, Maven, patch formats, etc.), but I'm willing to give it a shot. However, I would still need discussion on this topic (in Bugzilla, if more appropriate) before contributing a fix. What worked for me might not be generally applicable.

Thanks for looking into it.
Re: Bug in FilterRowDataProvider.{save,load}State()? [message #1018722 is a reply to message #1017947] Thu, 14 March 2013 11:04 Go to previous messageGo to next message
Dirk Fauth is currently offline Dirk Fauth
Messages: 1301
Registered: July 2012
Senior Member
Hi,

I digged deeper into the filter persisting stuff and it seems we are both right and wrong with our statements.

1. The filter is usually a String that gets converted into the object to filter by the converter that is registered with FilterRowConfigAttributes.FILTER_DISPLAY_CONVERTER

This isn't true for filters that are comboboxes, as in this case the objects will be handled usually.

2. Because of the combobox feature and the fact that custom types can not be translated into Strings easily (at least usually) the conversion is necessary.

Now for the big BUT:
The FilterRowDataProvider tried to convert the persisted Strings with the converter that is configured against FilterRowConfigAttributes.FILTER_DISPLAY_CONVERTER. This is of course wrong as this converter will be called on applying the filter (just as you mentioned). Changing this to the usage of the converter used for displaying the value, it seems to be working as it should for both cases. This is the converter registered for CellConfigAttributes.DISPLAY_CONVERTER.

From the configuration point of view this means you need to register the filter converter as well as the display converter for custom data types.
As there is fairly no documentation on the filtering stuff yet, we need to consider this when writing it down.

See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=403300

Greez,
Dirk
Re: Bug in FilterRowDataProvider.{save,load}State()? [message #1061479 is a reply to message #1018722] Fri, 31 May 2013 20:31 Go to previous message
Patrick Rusk is currently offline Patrick Rusk
Messages: 35
Registered: June 2012
Member
Dirk,

Thank you very much. I have upgraded to 1.0.0, and I've been able to get rid of the four classes I copied/modified to fix this issue. Your fix worked very well.

Pat
Previous Topic:Multi-cell editing changes between 0.9.0 and 1.0.0
Next Topic:Dependency on org.apache.commons.codec?
Goto Forum:
  


Current Time: Wed Oct 01 20:30:28 GMT 2014

Powered by FUDForum. Page generated in 0.10972 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software